Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify also when followed tests gets approved #2124

Open
peregrineshahin opened this issue Jul 28, 2024 · 10 comments
Open

Notify also when followed tests gets approved #2124

peregrineshahin opened this issue Jul 28, 2024 · 10 comments

Comments

@peregrineshahin
Copy link
Contributor

not an issue but nice to have. maybe can look to add at it later

@peregrineshahin
Copy link
Contributor Author

worth mentioning that a submitter follows his own tests automatically as of now

@peregrineshahin
Copy link
Contributor Author

An issue is that the folow=1 should be a post request instead of get, it makes no sense that if I send someone a link with ?follow=1 they follow the test???
need to be based on user actions not tricking them to follow

@vdbergh
Copy link
Contributor

vdbergh commented Jul 28, 2024

I can explain why it is that way. Normally one would want to follow a test after creating it. The problem however is that the server may reject a new run so that in that case one would be following a non-existing test.

Hence the ?follow=1 hack when the test page is loaded after creating the test (the fact that the test page is loaded means that the run is valid). I had not anticipated the people would be sharing these links.

Currently I think that the problem mentioned in the first paragraph is not really serious. Following a non-existing test is pretty harmless since after 4.5 hours it will be purged. Moreover, in most cases people will edit the new run form until a valid run is created anyway.

@vdbergh
Copy link
Contributor

vdbergh commented Jul 28, 2024

I will keep this issue in mind, but I am a bit hesitant to create new PRs at this time since there is already a sizable queue. I'd rather wait until the queue becomes a bit shorter.

@vdbergh
Copy link
Contributor

vdbergh commented Jul 29, 2024

Actually now I remember that there is a more fundamental reason for the ?follow=1 hack. When we submit a new run form, the run id isn't known yet. So we cannot start following the test when we submit. We really have to do it when the server sends us back the test page.

So the main question would be: what is a good alternative for ?follow=1? The server sending a cookie? Including some data in a meta tag?

@peregrineshahin
Copy link
Contributor Author

Better to keep the follow hack but change it to submited_test=1 and check here


if the user is the same as the submitter, that would be the easiest.

@peregrineshahin
Copy link
Contributor Author

peregrineshahin commented Jul 29, 2024

Or you can completely remove it and each time a user views his own test they start following it if they didn't.. and since you redirect to the test view immediately, the follow=1 is already redundant.

Edit: the difference in functionality would be that if they open the test from another device

@peregrineshahin
Copy link
Contributor Author

Ine trick also so that this works on all devices is most likely thee user will open the home page which if they did we can loop with js if they have any active tests or you can pass it from python.. then you can make it work on all devices by following them on this device

@vdbergh
Copy link
Contributor

vdbergh commented Jul 29, 2024

That's not a bad idea (I am not too worried about the slight difference in functionality).

We would need the name of the submitter of the current test. Since that name happens to be present on the test page, the easiest thing to do seems to just inspect the DOM using javascript (which admittedly is also a bit hacky).

@peregrineshahin
Copy link
Contributor Author

peregrineshahin commented Jul 29, 2024

Not neccessarly JS, python would be better ..each run viewed already has the submitter name and we have the authenticated_id, we can do run['args']['user'] == authenticated_id no?

Edit: the variables naming is off ofc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants