-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix/Edge browser - Invite invitation logic #1766
Conversation
lib/edge-utils.js
Outdated
|
||
// check if the invitation is an invite invitation | ||
export function isInvitationForInvite(invitation, headOrTail) { | ||
if (invitation[headOrTail]?.query?.['value-regex'] === '~.*|.+@.+') return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to check by regex because it may vary and sometimes it can not be present.
I would check if the invitation has inGroup
then show the button for the members of the group only.
If the invitation has notInfGroup
then show the button for the users that are not members of the group or the textbox.
Otherwise show the button everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
checking this regex is how it knows this is an "invite" invitation and show the button everywhere
otherwise any invitation that does not have inGroup and notInGroup will be invite invitation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about checking by the invitation name "Invite_Assignment"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there could be other invitations with the regex too and they care not invite assignment invitations?
…om/openreview/openreview-web into fix/edge-browser-invite-invitation
lib/edge-utils.js
Outdated
@@ -506,3 +511,18 @@ export function transformName(name, toVerb = false, pluralForm = false) { | |||
export function getInvitationPrefix(invitationId) { | |||
return invitationId ? invitationId.split('/-')?.[0] : null | |||
} | |||
|
|||
// check if the invitation is an invite invitation | |||
export function isInvitationForInvite(invitation, headOrTail) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are three scenarios here:
- invite not in group only
- invite in group only
- invite all
I would like to call it that way so it is clear that is associated with the keyword inGroup, notInGroup of the invitation
invitation[props.columnType]?.query?.['value-regex'] === '~.*|.+@.+' | ||
isInGroupInvite(invitation, props.columnType) || | ||
isNotInGroupInvite(invitation, props.columnType) || | ||
isForBothGroupTypesInvite(invitation, props.columnType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isInGroup||IsNotInGroup||IsForBothGroupTypes is essentially checking if the id has Invite_Assignment
written as || for clarity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me!
currently an invitation is treated as invite invitation only if it has the regex ~.*|.+@.+
this pr should update the logic to also include inGroup apart from regex which should fix case 1 and 2 described in #1761