Skip to content
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

Feature/autocomplete staff list no autocomplete #2

Merged
merged 13 commits into from
Mar 25, 2020

Conversation

andrewstec
Copy link

@andrewstec andrewstec commented Mar 24, 2020

Description: A staff user would attempt to add another staff user and experience a permissions error where an Autocomplete component would attempt to fetch user information that the staff role should not have access to.

Fix: Enable autocorrect for the admin user role, but only allow manual entries for the staff user role.

Some questions that I had in the PR draft (answered in comment below):

  • Is the filter working (and was it ever working) to filter out pre-existing users / admins
  • Where else is Userautocomplete used and did my changes break anything? (cannot tell by tests at the moment)
  • Do I need to connect and map the state to props in both components?
  • Are the prop shapes necessary, or even the best prop option?
  • Have the number of passing tests / failing tests changed?
  • Write unit tests.

@andrewstec
Copy link
Author

andrewstec commented Mar 25, 2020

  • Is the filter working (and was it ever working) to filter out pre-existing users / admins
    Tested on the master branch and it was not working. I am leaving the default behaviour.
  • Where else is Userautocomplete used and did my changes break anything? (cannot tell by tests at the moment)
    Tested the add admin view, and it works.
    Do I need to connect and map the state to props in both components?
    Do not have to, but it keeps things less confusing instead of passing it down and up components
    Are the prop shapes necessary, or even the best prop option?
    They are theoretically necessary, but errors are not being thrown when implemented as they should be. I implemented the description of the properties that I am using.
    Have the number of passing tests / failing tests changed?
    Master Branch:
    image
    Feature branch:
    image
    Yes
  • Fix unit tests.
    Yes, redefined parameters passed to back-end.
  • Write unit tests.
    Done for User not found feature.

@andrewstec andrewstec force-pushed the feature/autocomplete-staff-list-no-autocomplete branch from b89d6e7 to 380715b Compare March 25, 2020 17:04
Copy link
Member

@winstan winstan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants