Support Authgear Once user provision #4937
Merged
Conversation
This file contains hidden or 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
tung2744
reviewed
Dec 11, 2024
Comment on lines
+1
to
+15
| # The use of variables | ||
| # | ||
| # We use simply expanded variables in this Makefile. | ||
| # | ||
| # This means | ||
| # 1. You use ::= instead of = because = defines a recursively expanded variable. | ||
| # See https://www.gnu.org/software/make/manual/html_node/Simple-Assignment.html | ||
| # 2. You use ::= instead of := because ::= is a POSIX standard. | ||
| # See https://www.gnu.org/software/make/manual/html_node/Simple-Assignment.html | ||
| # 3. You do not use ?= because it is shorthand to define a recursively expanded variable. | ||
| # See https://www.gnu.org/software/make/manual/html_node/Conditional-Assignment.html | ||
| # You should use the long form documented in the above link instead. | ||
| # 4. When you override a variable in the command line, as documented in https://www.gnu.org/software/make/manual/html_node/Overriding.html | ||
| # you specify the variable with ::= instead of = or := | ||
| # If you fail to do so, the variable becomes recursively expanded variable accidentally. |
Contributor
There was a problem hiding this comment.
Did you find any problem with the original ?= recursively expanded variable?
Contributor
Author
There was a problem hiding this comment.
No problem in our current usage. I just learnt this today and realized we are not very consistent with the use of the variables. We never intent to use recursively expanded variables.
tung2744
reviewed
Dec 11, 2024
pkg/portal/service/collaborator.go
Outdated
Comment on lines
484
to
496
| inviteeExists, err := s.checkInviteeExistenceByEmail(ctx, invitedBy, inviteeEmail) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| if !inviteeExists { | ||
| err = s.createAccountForInvitee(ctx, invitedBy, inviteeEmail) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
New developers (If any) reading this part may be confused:
Why createAccountForInvitee is called but actually no account created?
What about:
// In CollaboratorService.SendInvitation
if IsAuthgearOnce {
// ... checkInviteeExistenceByEmail
// ... createAccountForInvitee
}
// In another file
//go:build !authgearonce
// +build !authgearonce
const IsAuthgearOnce = false
// In another file
//go:build authgearonce
// +build authgearonce
const IsAuthgearOnce = true
Contributor
Author
There was a problem hiding this comment.
Agree, let me make the change according to your suggestion.
louischan-oursky
force-pushed
the
dev-2341-once-user-provision
branch
from
b11c96d to
12cdeec
Compare
tung2744
approved these changes
Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
ref DEV-2341