Modified edit-person script to accept WCA ID via query param and search option in country picker #8975
Conversation
| @@ -0,0 +1,30 @@ | |||
| import { useEffect, useState } from 'react'; | |||
|
|
|||
| function getQueryParamsFromBrowserUrl() { | |||
There was a problem hiding this comment.
Can you use Object.fromEntries here?
There was a problem hiding this comment.
Wow that's easier, changed.
|
|
||
| useEffect(() => { | ||
| const searchParams = new URLSearchParams(queryParams).toString(); | ||
| if (window.location.search !== searchParams) { |
There was a problem hiding this comment.
!== checks for referential equality. Since you are creating a new URLSearchParams object directly above, I feel like this comparison always yields true because they can by defintion never be the same.
There was a problem hiding this comment.
I'm acutually converting it to string, but looks like window.location.search is having a prefix '?' when there is at least one param. I've handled that case as well. Now it's working fine I guess.
There was a problem hiding this comment.
This approach looks like code duplication. I feel like we've dealt with this private attribute notion before, can you find a way to re-use the existing code?
There was a problem hiding this comment.
I tried, but couldn't find any way. The other place where it is used is in api_controller and there the result can be of any model - person or user or something else. Here, I want just the person model. The only way I can think of re-using is moving the check current_user && current_user.can_admin_results? to serializable_hash, which I think will make the serializable_hash more complex. Do you have any idea?
danieljames-dj
changed the title
|
Added search option in country picker as selecting country is tough now |
danieljames-dj
force-pushed
the
edit-person-fix
branch
from
c3b6b37
to
8e8a99c
Compare
| { | ||
| person: person.serializable_hash(only: [:wca_id, :name, :url, :gender, :country_iso2, :delegate_status, :teams, :avatar]), | ||
| person: person.serializable_hash(only: [:wca_id, :name, :url, :gender, :country_iso2, :delegate_status, :teams, :avatar, private_attributes].flatten), |
There was a problem hiding this comment.
I think this should be person.serializable_hash(only: [...], private_attributes: [...]) so that the existing code in person.rb can pick it up.
| function updateQueryParam(key, value) { | ||
| setQueryParams({ | ||
| ...queryParams, | ||
| [key]: value, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Should this be wrapped in useCallback?
No description provided.