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

Users API #60

Merged
merged 21 commits into from
Jun 28, 2021
Merged

Users API #60

merged 21 commits into from
Jun 28, 2021

Conversation

edarchis
Copy link
Member

Add the various Users APIs

edarchis and others added 16 commits May 17, 2021 23:34
The users query is now querying over the ClaimAdmin and Officer types.

This is not really a clean solution but it is the best that we could make with the existing table structures without breaking everything else.
Be careful not to optimize the graphql query or it would prevent the User dynamic mapping to the actual objects because some of them would be marked as "deferred".
The Mutations are not yet part of this commit but will follow shortly.
This requires an update to the Location module.
The "user" in UserMutation was clashing with the operating user.
It is now possible to call the user creation/update method with just the username instead of the exact core_user ID
health_facility_id was of the wrong type and not rendered in the response for InteractiveUser because the field is not a foreign key. Also added roles with a direct rendering rather than through a userRoles intermediate object.
The mutation link to actual objects was performed on the actual object itself. In the case of User, it caused some conflicts with its `__call__` method because _u was not properly initialised from Django itself. So we are now adding _id to the object and passing its id. We still need the object itself to identify the foreign key.
UserMutation was also not exposed to GraphQL and the user was not properly linked for the mutation, it was linked to connected user.
This was preventing the delete of users created from the mutations.
OFS-290: fixed bug with performing uuids bulk actions
@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2021

This pull request fixes 1 alert when merging 549ee51 into 1f9a687 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2021

This pull request fixes 1 alert when merging 4ccb3f8 into 1f9a687 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Jun 16, 2021

This pull request fixes 6 alerts when merging 8d26a2c into 1f9a687 - view on LGTM.com

fixed alerts:

  • 3 for Unused local variable
  • 3 for Unused import

@edarchis edarchis marked this pull request as ready for review June 16, 2021 14:56
@lgtm-com
Copy link

lgtm-com bot commented Jun 25, 2021

This pull request introduces 2 alerts and fixes 6 when merging 70d90d5 into 1f9a687 - view on LGTM.com

new alerts:

  • 2 for Use of a broken or weak cryptographic hashing algorithm on sensitive data

fixed alerts:

  • 3 for Unused local variable
  • 3 for Unused import

@edarchis
Copy link
Member Author

Weak cryptography warning from LGTM is partly valid (SHA256 is not bad per se) but is for compatibility reasons.

@edarchis edarchis merged commit 826974f into develop Jun 28, 2021
@edarchis edarchis deleted the feature/users branch June 28, 2021 13:47
on ClaimAdminCode=cu.username
where cu.claim_admin_id is null;
""", reverse_sql="")
]
Copy link
Member

Choose a reason for hiding this comment

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

@dragos-dobre I don't think we need to do anything but modular is using FK for admin/EO (Which makes sense) but if you really want to align legacy then some action are required

# TODO make a signal to contract module to not break modular approach and
# not repeat the same code from contract module

status_notified = [1, 2]
Copy link
Member

Choose a reason for hiding this comment

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

These numbers are difficult to understand. What is their meaning? Should you create meaningful constants and use them here?

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.

5 participants