Backend with accounts and province ranking system #50
Conversation
This reverts commit 68f70cf.
| championship.province = competition.province | ||
| championship.competition = competition.key | ||
| championship.put() | ||
| # TODO: if we changed a championship we should update champions and eligibilities. |
There was a problem hiding this comment.
If these TODO comments aren't going to be addressed in this PR, please open an Issue and reference it in the comment here.
| # Cache mapping of province names. | ||
| provinces_by_name = {} |
There was a problem hiding this comment.
Is a cache really necessary? Seems like it's possibly premature optimization and we would benefit more from making the getter simpler.
There was a problem hiding this comment.
I get your point but I think we have few provinces and a lot of references to them it actually would save us a ton of request to keep the cache .If we remove it we will be querying the datastore a lot more to always get the same few results.
| maybe_province = Province.get_by_id(province_name.replace('.', '').replace(' ', '').lower()) | ||
| if not maybe_province: | ||
| # Or maybe it's the name. | ||
| maybe_province = Province.query(Province.name == province_name).get() | ||
| if maybe_province: | ||
| provinces_by_name[province_name] = maybe_province | ||
| return maybe_province |
There was a problem hiding this comment.
| maybe_province = Province.get_by_id(province_name.replace('.', '').replace(' ', '').lower()) | |
| if not maybe_province: | |
| # Or maybe it's the name. | |
| maybe_province = Province.query(Province.name == province_name).get() | |
| if maybe_province: | |
| provinces_by_name[province_name] = maybe_province | |
| return maybe_province | |
| province_by_id = Province.get_by_id(province_name.replace('.', '').replace(' ', '').lower()) | |
| if province_by_id: | |
| return province_by_id | |
| # Or maybe it's the name. | |
| province_by_name = Province.query(Province.name == province_name).get() | |
| if province_by_name: | |
| return province_by_name | |
| return None |
| client = ndb.Client() | ||
|
|
||
|
|
||
| #@bp.route('/add_championship/<competition_id>/<championship_type>') |
There was a problem hiding this comment.
I see. Please still make an Issue as per the other comment. (TODO comments in the code tend to get ignored more than Issues.)
There was a problem hiding this comment.
In the admin section, it says "1-10 of more than 10" for the pagination (there are 13 results, 10 of them visible), and then the next page says "11-20 of 20" (still 13 results of course, and 3 visible). It's just the admin page so it doesn't matter, but if there's a simple fix to "1-10 of 13" and "11-13 of 13" then that would be good.
The UI also briefly shows "No users yet" while loading, which is incorrect. Again, it's the admin page so maybe that's good enough.
Still on the admin page, improving the search to work with any part of the name would be ideal, not sure how easy that would be.
I still think the red theme is bad because all the inputs look like they're in an error state after I input something, but if nobody else has any problem with it then it's fine.
| getManyReference: ( | ||
| resource: any, | ||
| params: { | ||
| //not setup |
There was a problem hiding this comment.
Is this something that needs to be done?
There was a problem hiding this comment.
Not really. I mean react-admin could maybe call it but I don't think our current use really needs to implement all the methods.
|
More admin page stuff: if I select 5 rows per page then it just shows all 13 rows. |
I fixed it |
|
To answer the comments about the user list in the admin section. Currently are limited by outside factors. For pagination and searching, I will write an issue to describe that more precisely. (edit, done: #56 ) For the red part, I understand your concern but also it's doesn't shock me at all so I would like feedback from other people to see how they feel. |
AlexandreOndet
changed the base branch from
9_future_routes
to
backend+accounts+rankings
This PR is a new version #45, I made a new branch to showcase the new changes since the last code review, you can see them at : AlexandreOndet@db8642c