FAI-CIVL / FAI-Airscore

AirScore - online paragliding / hanggliding GAP-based scoring software.
https://airscore.cc/
GNU General Public License v3.0
13 stars 17 forks source link

Overall results automatically published on task result publish (bug) #178

Closed kuaka closed 3 years ago

kuaka commented 3 years ago

in testing #163 I noticed that overall comp results were automatically being generated and published when scoring a task and publishing. Previously airScore did not behave this way. IMHO this is not desired behavior, a score keeper may want to publish provisional task results and not have them affect the overall result until final.

Also deleting this overall result did not remove them from display.

biuti commented 3 years ago

Usually you publish provisional task and provisional comp results together.What could be useful is a flag that indicates if results are final or not, and comp results to be indicated as final only if all task are scored and flagged final.

kuaka commented 3 years ago

Doesn't make much sense to me. If that is the case the UI to publish comp results can be removed as it is automatic.

I can't understand why you would want to update overall results when for example only 50% of pilots have been processed for the latest task.

On Sun, 22 Nov 2020, 22:34 Antonio Golfari, notifications@github.com wrote:

Usually you publish provisional task and provisional comp results together.What could be useful is a flag that indicates if results are final or not, and comp results to be indicated as final only if all task are scored and flagged final.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/FAI-CIVL/FAI-Airscore/issues/178#issuecomment-731721382, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETWEH7BKMLOQX54RBYL75TSRDLJJANCNFSM4T5PP7BA .

biuti commented 3 years ago

No, only if they are provisional, that means with all tracks scored. That's why we need a flag. Usually partial results are not published in the same format, as they depend from the missing ones. They should be published as in telegram, a list of pilots, distance / time, and link to map. That could be a nice improvement for the future. By now, I agree, we can either use a flag and a logic as I wrote or use a similar interface for comp result as task result is, so to decide if it is publish or not, and a status to fill.

kuaka commented 3 years ago

..or use a similar interface for comp result as task result is, so to decide if it is publish or not, and a status to fill.

This is how it used to be. I raised this issue as I was unaware that there was a decision to change it and presumed it was another bug. This way makes sense to me, today and back when it was implemented, and seems to still be reflected in the UI. i.e. the update button is still there.

biuti commented 3 years ago

I don't remember the php version, in the LP website it is like this. So no option to publish or not and no status. It makes sense to let scorekeeper decide tho. So I'll change it to have same logic as task result. This way probably we can avoid a flag in resultfile table, even if I think it could be useful to have.

image

kuaka commented 3 years ago

The screen shot has remove and update. That was the previous logic that I wrote. The time that the comp results were updated is different from the time that the task was scored. Therefore not automatic. This is how it should be.

On Sun, 22 Nov 2020, 23:10 Antonio Golfari, notifications@github.com wrote:

I don't remember the php version, in the LP website it is like this. So no option to publish or not and no status. It makes sense to let scorekeeper decide tho. So I'll change it to have same logic as task result. This way probably we can avoid a flag in resultfile table, even if I think it could be useful to have.

[image: image] https://user-images.githubusercontent.com/35658597/99900941-4139b080-2cb3-11eb-99fa-a8564e597ffb.png

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/FAI-CIVL/FAI-Airscore/issues/178#issuecomment-731724969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETWEH2JMGHWMTS7DJSGKO3SRDPRNANCNFSM4T5PP7BA .

kuaka commented 3 years ago

this is the commit that I believe created this bug: 57974b79fcc012e573244ce07569f2d1da47fbef

kuaka commented 3 years ago

the old logic was: if task is published, automatically create a comp result that is an 'overview'. This overview is not a comp result that is displayed at the front end. It is only used to generate the competition overview page (/competition/). This file, and therefore page will reflect the latest published task result data but the overall competition results will not be updated and published unless the scorekeeper chooses. We do this for speed, when we request the page we know the compid so it is fast to load the overview data. Otherwise we would have to query DB, get task IDs and look for all result files and build the page. This is the best logic IMHO.

biuti commented 3 years ago

Ok, I will keep creating the overview file, while using for comp results a similar approach to task results.

kuaka commented 3 years ago

Ok, I will keep creating the overview file, while using for comp results a similar approach to task results.

just to be clear, this is the old logic. So it may be easier just to roll back and address the original bug that https://github.com/FAI-CIVL/FAI-Airscore/commit/57974b79fcc012e573244ce07569f2d1da47fbef addressed. Just needed to fail in a safe way or have a work around so it didn't fail. No point in re-inventing the wheel here.

biuti commented 3 years ago

commit https://github.com/FAI-CIVL/FAI-Airscore/commit/a8a8d34da31e1f1c1b99a7fb2d94496966ec7947 should have corrected this behaviour. Now in Comp result tab the user is advised that Auto generated file is kept updated with task activated results. image

kuaka commented 3 years ago

I find this a bit odd.

The "overview" file was never intended for results. It was the easiest way to have the task info available on the competition overview page even if the competition results had not been generated/refreshed when the task was scored. This is purely a backend process... the user should not care or be aware of. As far as I know they don't even know that the results are stored in a file... for all they know it could be in the database, it doesn't really matter..it is just a backend process..

I find this whole issue bizzare.. we had a process that was working fine. It got broken with a "bug fix" now the process has been re engineered and complicated that even the user is now informed of the backend workings of airscore and has even more decisions to make (and be trained on)... whereas we could have simply fixed what was broken in the original bug fix and moved on to more important things..

kuaka commented 3 years ago

The "overview" file was never intended for results. It was the easiest way to have the task info available on the competition overview page even if the competition results had not been generated/refreshed when the task was scored.

*we could have created a new file or DB entry just with the info that the comp overview page needed. Task map, date,distance, status, etc but at the time it was easier just to produce an overview file. The competition page (overview page) needs to use the latest comp result file whether it is published or just an overview file. So they have to be compatible, hence the reason just to use the exact same result file format.

kuaka commented 3 years ago

@biuti can you enlighten me on what problem this big refactor addressed? I am still puzzled.

biuti commented 3 years ago

As it was before there was no possibility to keep daily result files, or they were saved but it was not possible to reach them from frontend. Now scorer has full control exactly as in Task results. The Auto Generated file is just a feature for simple events, where there is no need to have discrepancies between activated task results and comp results. We already were generating it anyway, and it cannot be edited by user apart from Status. Still I'd like to have a list of computed tasks selecting a comp file, but now at least there is a preview so user can check results before publishing. As layout is identical with task tab, I'm not convinced it is complicated.

kuaka commented 3 years ago

ok. personally I think it is overly complicated in that a comp score should be either equal to the sum of all other scores or only the scores that are official. Now we can have a situation where comp is mismatched with task scores. In the past there was no issue just pressing "update" if we decided to re-acitvate old task scores etc. it was fool proof, now we are asking the scorer if he reactivates an old task result to try and work out which comp result he should activate. Am closing now. I am always puzzled by these big refactors that end up changing very little and add bugs along the way. Your choice how you spend your time but from my point of view I see a project with serious bugs and lots and lots of untested code and we spend time on big changes to things that are working fine. It's like part of the house having a missing roof and we spend time repainting one of the rooms.. just my 2 cents.