Ms2/signin merge users #360
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change was needed, because the function was redundant and was implemented in a way that caused it to fail. It is redundant to use a userId and an ability, because both tell you for which user we're supposed to get the processes. And it fails because, it returns the processes where the creator was the user, this is wrong, because it doesn't take into consideration abilities. For me is better that the function takes in a spaceId, and returns all processes for a spaceId, and if you want to get the processes for a user you can provide his ability.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
OhKai
reviewed
Dec 6, 2024
| updateFolderMetaData, | ||
| updateProcess, | ||
| } from '@/lib/data/DTOs'; | ||
| import { deleteUser, getUserById } from '@/lib/data/legacy/iam/users'; |
There was a problem hiding this comment.
Are the DB methods still TODO?
There was a problem hiding this comment.
What do you mean?
There was a problem hiding this comment.
Oh, they're already implemented, the import was just wrong.
I updated it.
src/management-system-v2/app/transfer-processes/transfer-processes-confitmation-buttons.tsx
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
✅ Successfully created Preview Deployment. |
OhKai
approved these changes
Dec 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Allow processes created by a guest user to be transferred to an already existing authenticated user.
Details
Flow for transferring processes.
referenceTokenthat stores the guestId and passes it to the client componentreferenceTokenis used in the callbackUrl of the authentication flowUpdate to
getProcessesget processes takes in an
(spaceId, ability), instead of(userId , ability).This change was needed, because the function was redundant and was
implemented in a way that caused it to fail.
It is redundant to use a userId and an ability, because both tell you
for which user we're supposed to get the processes.
And it fails because, it returns the processes where the creator was the
user, this is wrong, because it doesn't take into consideration
abilities.
For me is better that the function takes in a spaceId, and returns all
processes for a spaceId, and if you want to get the processes for a user
you can provide his ability.