feat: user office automatic joining of invitees to the #1260
Conversation
yoganandaness
requested review from
simonfernandes
and removed request for
a team
yoganandaness
changed the title
|
simonfernandes
left a comment
simonfernandes
left a comment
There was a problem hiding this comment.
Just some minor comments/questions. We won't be using this at STFC but it looks like a cool feature 🤩
| if (!agent) { | ||
| return rejection('User not found', { proposalId }); | ||
| } |
There was a problem hiding this comment.
This could be done before getting the proposal
| return rejection('Invite not found', { proposalId }); | ||
| } | ||
|
|
||
| const updatedInvite = await this.inviteDataSource.update({ |
There was a problem hiding this comment.
Would it be safer to update the invite after the processing of the claims below?
There was a problem hiding this comment.
@simonfernandes Absolutely. It make sense to update after processing the claims.
It was earlier in this way. i changed it to fix the issue, where the email handler was not able to fetch the claimedByUserId, as it was not updated to the invite. So i fixed the issue by flipping the logic, so that invite is updated first.
I now get the importance of the order and realized this could be resolved in other way as well, which i have done in this PR in essEmailHandler.
Thanks @simonfernandes
| {invite.proposal.title || 'No Title'} | ||
| </Typography> | ||
| <Typography variant="body1" color="textSecondary"> | ||
| Principal Investigator: {invite.proposal.proposerName} |
There was a problem hiding this comment.
If inviting others is not restricted to the proposer, is it useful to show who has sent the invite?
There was a problem hiding this comment.
We are already including the information in the Email about the inviter. So, thought it would be minimal to just include the Proposal number and the PI and abstract away all the other information.
However, It is a good point and i will discuss with the team and reserve this for a later PR, if it is fine for you.
There was a problem hiding this comment.
Yes of course, just thought to mention it in case it would be useful, but if there's no practical use for it then there's no point adding it for the sake of it 🙂
| FROM invites | ||
| WHERE email = '[email protected]' AND code = 'BEN002'; | ||
|
|
||
| INSERT INTO co_proposer_claims (invite_id, proposal_pk) VALUES (invite_id_var3, 2); |
There was a problem hiding this comment.
I wonder why we actually need this table. Is it possible to create a single invitation for multiple proposals? If not, proposal_pk could be part of invites table.
There was a problem hiding this comment.
It is not yet likely that an invite could be used for more than one proposal. However, the whole design is it make sure that the invite system works not just for entity like Proposal, but also for other entities like Reviewers, Instrument Scientist, Visits etc.,
github-actions
bot
force-pushed
the
SWAP-4914-user-office-automatic-joining-of-invitees-to-the
branch
from
8fb99c8 to
089815a
Compare
github-actions
bot
force-pushed
the
SWAP-4914-user-office-automatic-joining-of-invitees-to-the
branch
from
089815a to
cdc2577
Compare
…f-invitees-to-the
…f-invitees-to-the
Description
Allow co-proposer invitees to accept their invitation and join a proposal without needing to enter an invite code. This streamlines the process for co-proposers, making it easier for them to join proposals they have been invited to.
Motivation and Context
Previously, co-proposer invitees were required to enter an invite code to accept their invitation and join a proposal. This change removes that requirement, improving the user experience and reducing friction for co-proposers joining proposals.
How Has This Been Tested
InviteMutations.spec.tsto cover the new flow.invites.cy.ts.Fixes
Fixes part of the user story for automatic joining of invitees as co-proposers.
Changes
InviteMutations.tsto allow co-proposer invites to be accepted without a code.InviteMutations.spec.tsand e2e tests ininvites.cy.ts.AcceptInvite.tsxto support the new flow.Depends on
N/A
Tests included/Docs Updated?