feat: allow inviting Data Access Users to Proposal with email#1615
feat: allow inviting Data Access Users to Proposal with email#1615shivoomiess wants to merge 23 commits into
Conversation
yoganandaness
left a comment
There was a problem hiding this comment.
@shivoomiess Kindly attach relevant video or screenshot
yoganandaness
left a comment
There was a problem hiding this comment.
- kindly test all pieces of change
- Attach video or pic if possible
- Pls have clean diff
jekabs-karklins
left a comment
There was a problem hiding this comment.
Over all looks very good :raisedhands:
As this is a bigger PR I think you will get a bit more comments :).
I have checked the PR and left some comments, and Happy to chat if any help or clarification needed.
shivoomiess
left a comment
There was a problem hiding this comment.
incorporated feedback
| try { | ||
| await this.processAcceptedRoleClaims(agent.id, invite); | ||
| await this.processAcceptedCoProposerClaims(agent.id, invite); | ||
| await this.processAcceptedDataAccessClaims(agent.id, invite); | ||
| await this.processAcceptedVisitRegistrationClaims(agent.id, invite); | ||
| } catch (error) { | ||
| logger.logException('Error during claim processing', error); | ||
| } | ||
|
|
There was a problem hiding this comment.
breaking: the problem is that catch will capture the error, and flow will continue, and it will mark invite as claimed. We need to re-throw or simply remove the try...catch block all together.
The any exception occurring are handled globally under apps/backend/src/middlewares/graphql.ts see errorLoggingPlugin
| async addDataAccessUser( | ||
| proposalPk: number, | ||
| userId: number | ||
| ): Promise<undefined | Rejection> { | ||
| try { | ||
| await database.transaction(async (trx) => { | ||
| const insertData = { | ||
| proposal_pk: proposalPk, | ||
| user_id: userId, | ||
| }; | ||
|
|
||
| await database('data_access_user_has_proposal') | ||
| .insert(insertData) | ||
| .onConflict(['proposal_pk', 'user_id']) | ||
| .ignore(); | ||
| }); | ||
| } catch (error) { | ||
| return new Rejection('Failed to add data access user', { | ||
| proposalPk, | ||
| userId, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion: there is no need for transaction here as we only do one database mutation
| async addDataAccessUser( | |
| proposalPk: number, | |
| userId: number | |
| ): Promise<undefined | Rejection> { | |
| try { | |
| await database.transaction(async (trx) => { | |
| const insertData = { | |
| proposal_pk: proposalPk, | |
| user_id: userId, | |
| }; | |
| await database('data_access_user_has_proposal') | |
| .insert(insertData) | |
| .onConflict(['proposal_pk', 'user_id']) | |
| .ignore(); | |
| }); | |
| } catch (error) { | |
| return new Rejection('Failed to add data access user', { | |
| proposalPk, | |
| userId, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| } | |
| } | |
| async addDataAccessUser(proposalPk: number, userId: number): Promise<undefined | Rejection> { | |
| try { | |
| await database('data_access_user_has_proposal') | |
| .insert({ proposal_pk: proposalPk, user_id: userId }) | |
| .onConflict(['proposal_pk', 'user_id']) | |
| .ignore(); | |
| } catch (error) { | |
| return new Rejection('Failed to update data access users', { | |
| proposalPk, | |
| userIds, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| } | |
| } |
| event, | ||
| }); | ||
|
|
||
| return; |
There was a problem hiding this comment.
breaking: should be continue instead of the return, so that in case the user is missing for one invite we do not skip the rest of the invites. I can see this bug originates by taking the code example from above. Could you also address the issue in the example code?
| if (filter.isExpired) { | ||
| query.where('expires_at', '<', new Date()); | ||
| } |
There was a problem hiding this comment.
breaking: this currently checks if isExpired is a truthy value then apply the filter.
In VisitMutations we actually use this param to filter out expired invites by specifying { isExpired: false }, which looks right from outside but as this is implemented will skip the check alltogether, meaning you can accept expired invites.
Again I see this originating from the example code, the fix would look something like this
| if (filter.isExpired) { | |
| query.where('expires_at', '<', new Date()); | |
| } | |
| if (filter.isExpired !== undefined) { | |
| if (filter.isExpired) { | |
| query.where('expires_at', '<', new Date()); | |
| } else { | |
| query.where((qb) => | |
| qb.whereNull('expires_at').orWhere('expires_at', '>=', new Date()) | |
| ); | |
| } | |
| } | |
Description
This PR introduces the capability to invite data access users to a proposal by creating a new data access claim.
Motivation and Context
This change is required to allow data access users to be linked directly to proposals, enabling them to view and manipulate data related to the proposal they are invited to. This enhances collaboration and data management in the project.
Changes
Visual Changes
The invite pop-ups have been updated to look clean and show the type of proposal invite (Co-Proposer / Data Access).
Previous
Current
How Has This Been Tested?
Adding and Deleting Invited Users
Screen.Recording.2026-07-02.at.19.51.10.mov
Screen.Recording.2026-07-02.at.19.51.42.mov
Screen.Recording.2026-07-02.at.19.53.53.mov
Invite Popups for New Sign-Ups
Screen.Recording.2026-07-06.at.16.15.20.mov
Fixes Jira Issue
https://jira.ess.eu//browse/SWAP-5408
Depends On
Tests included/Docs Updated?