official-stockfish / fishtest

The Stockfish testing framework
https://tests.stockfishchess.org/tests
280 stars 129 forks source link

Notify also when followed tests gets approved #2124

Open peregrineshahin opened 2 months ago

peregrineshahin commented 2 months ago

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

peregrineshahin commented 2 months ago

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

peregrineshahin commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Better to keep the follow hack but change it to submited_test=1 and check here https://github.com/official-stockfish/fishtest/blob/12981ffad22f5e44420d40ea9298b6ff83c0bfc2/server/fishtest/templates/tests_view.mak#L29 if the user is the same as the submitter, that would be the easiest.

peregrineshahin commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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