-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add multi-key sorting of statuses #571
Comments
The above is my feeling about what needs to happen for this to work; but I could be wrong. I don't believe it would be possible to calculate the sort index dynamically when calling the datatables method. Reason: datatables needs to get the sort index to do its server-side sorting, so it would really need all of those values present. Having that data cached in the table is the only way to do it, afaict. |
While this change is not simple, it's pretty easy, so I think it could be tackled by any motivated dev who wants to give it a shot. |
@jzohrab I'm definitely motivated and willing to give this one a go if nobody objects - worst case I learn more about the codebase, right? If so, I'd appreciate your input on the best way to test the migrations - or do you just test the end result? Thanks! |
Hey @parradam -- In this case, the migration itself doesn't touch existing data (phew!), it just adds a new field, and then some new code will populate that. That new code needs the tests, not the migration itself. The migration itself will be a single-line text file, something like |
You can push things as you go to a Draft PR and I can take a look if you'd like. I don't have a pile of time but can def look. :-) |
Happy to give it a go! |
From discord link
I guess it should sort by unknowns then status 1 through 5
Notes
The book listing is in
lute/templates/book/tablelisting.html
, with the graph column first getting theUnknownPercent
from the datatables query inlute/book/datatables.py
, so the unknown percent is used for sorting. The graph is then rendered "on top" of that, withajax_in_book_stats
in the html.The
UnknownPercent
is pulled from the tablewith values like this:
"58" here is the percent of unknown terms.
To add this feature correctly, the code needs to change the single value "58" to a composite key (in this case something like
{"0": "048", "1": "023", "2": "011", "3": ... etc }
, with three digit places for each element to allow for100
in one of the slots).Doing all of this in the course of a DB migration would maybe require a python script to run, to load the status_distribution json string and then do calculations. The migration tool currently doesn't handle such a thing. Instead there could be a data clean-up job that runs at startup, that checks the content of the sort string, and if it doesn't match the required pattern it could do some fast processing.Updated spec after discord discussion.
Probably the best way to do this is to have a method
calc_sort_keys
added tolute/book/stats.py
which does the calc for a newstatus_distribution_percent
field if it's null and if thestatus_distribution
is not null. Call this on every call to the datatables function so the key is always updated.todos I can think of
consider if can be linked somehow to A better way to sort by difficulty #453-- no don't bother, stick with the current statestatus_distribution_percent
, varchar(100). No need to drop the "unknownpercent" column, it's ok to leave that for later.lute/book/stats.py
, addcalc_sort_keys
method to loadstatus_distribution_percent
. Unit tests: nulls, bad json in the distribution field, empty string, valid distribution, distribution with 100% and 0%calc_sort_keys
in the book datatables pythoncalc_sort_keys
unknownpercent
column as it's no longer usedThe text was updated successfully, but these errors were encountered: