Skip to content
This repository was archived by the owner on May 15, 2023. It is now read-only.

Conversation

@oliverpool
Copy link
Contributor

@oliverpool oliverpool commented Feb 29, 2020

The creation of a participant is now split in 2 steps: first indicate the NaMi number and then eventually complete the participant data:

  • If the nami number is already in our db, we redirect to this user (except if it belongs to another troop), otherwise we perform a lookup against the NaMi API (with the troop number to prevent leaking personal info from troops not managed by the user).
  • If the lookup successful, the participant creation form is prefilled with some data.
  • If the lookup fails, the participant creation form is displayed empty (except for the nami number)

The design is made to make a few requests as possible against the NaMi API:

  • if user already in our db, no request is done
  • otherwise only 1 request is performed (the search)

Moreover this feature is quite heavy on privacy : the search is restricted to NaMi number belonging to the current troop.

And lastly, the implementation tries to be robust: on API failure, the empty form is still displayed (and everything is unit-tested ;-)

@oliverpool oliverpool changed the title Add Nami lookup Add NaMi lookup Feb 29, 2020
Copy link
Member

@ackxolotl ackxolotl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed

@ackxolotl
Copy link
Member

Some more ideas on this PR:

  • Add batch import functionality (using a list of NaMi members)
  • Allow to lookup participants by first name, last name and birthday instead of membership number (NaMiSearchView)
  • Get age section and is-leader from NaMi when importing a participant

@oliverpool
Copy link
Contributor Author

Good ideas!

Add batch import functionality (using a list of NaMi members)

I think that should be discussed in another Issue (or MR) to keep this one as small as possible.
(I think it will probably hard to find the right balance between privacy and few requests to the NaMi, but we can give it a try).

Allow to lookup participants by first name, last name and birthday instead of membership number

Probably useless if your first idea is implemented :-)

Get age section and is-leader from NaMi when importing a participant

  • Technical: According to my tests, this info is not returned on a search request (so it will require a second request to the NaMi, which is a bit uncool)
  • UX: I don't think that checking 2 boxes is hard (or error-prone), so the StaVos will not gain so much time from this feature
  • NaMi: this info might not be up-to-date in the NaMi and introduce errors if the StaVo don't double-check this.

For those reasons, I would let it as-is.

@ackxolotl
Copy link
Member

  • Technical: According to my tests, this info is not returned on a search request (so it will require a second request to the NaMi, which is a bit uncool)
  • UX: I don't think that checking 2 boxes is hard (or error-prone), so the StaVos will not gain so much time from this feature
  • NaMi: this info might not be up-to-date in the NaMi and introduce errors if the StaVo don't double-check this.

For those reasons, I would let it as-is.

entries_stufe should do the job (or at least it did – depending on your privileges I think). I think it would be a good incentive for StaVos to keep NaMi up-to-date ;)

@oliverpool
Copy link
Contributor Author

oliverpool commented Mar 2, 2020

On my tests, entries_stufe is present on troop-list (mitglied/filtered-for-navigation/gruppierung/gruppierung/XXXX/flist), but empty on search (search-multi/result-list).

@ackxolotl
Copy link
Member

Hm, ok 🤷‍♂️ Then let's keep it as it is (at least for now).

@oliverpool oliverpool requested review from 6543 and elnappo March 2, 2020 18:50
@oliverpool oliverpool merged commit d0230a4 into master Mar 10, 2020
@oliverpool oliverpool deleted the nami branch March 10, 2020 08:53
@oliverpool oliverpool mentioned this pull request Mar 29, 2020
13 tasks
ackxolotl pushed a commit that referenced this pull request Mar 29, 2020
ackxolotl pushed a commit that referenced this pull request Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants