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

Ticket-547 updated sub-user service and controller to use user entity #568

Merged

Conversation

ckirby19
Copy link
Contributor

@ckirby19 ckirby19 commented Aug 1, 2024

Issue link / number:

#547

What changes did you make?

Replace use of req["user"] with req["userEntity"] and addressed other issues where needed

Why did you make the changes?

As requested in the ticket as part of a larger refactor

Did you run tests?

Yes, unit and e2e tests

@kyleecodes kyleecodes self-requested a review August 5, 2024 21:15
Copy link
Member

@annarhughes annarhughes left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this issue @ckirby19 🙏 One type issue to fix and it's ready to be merged!

req['user'].user.id,
req['user'].user.email,
req['userEntity'].user.id,
req['userEntity'].user.email,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
req['userEntity'].user.email,
req['userEntity'].id,
req['userEntity'].email,

the new userEntity object doesn't have a child user object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yes apologies, I have changed that now

@annarhughes annarhughes self-assigned this Aug 14, 2024
@eleanorreem
Copy link
Contributor

Thanks @ckirby19 looks great 😄

@eleanorreem eleanorreem merged commit 8ed2260 into chaynHQ:develop Aug 20, 2024
3 checks passed
@ckirby19 ckirby19 deleted the refactor/ticket-547-replace-req-user branch August 20, 2024 18:28
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.

3 participants