From 90e5c2886db596fcb1d2e301f922e1f257d478b6 Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Fri, 12 Jul 2024 12:00:26 +0300 Subject: [PATCH] Merge pull request #19249 from overleaf/ii-invite-token-response [web] Fix share modal resend status code GitOrigin-RevId: 303f7b6c49d9522df6317789bb7c3c69d774715f --- .../CollaboratorsInviteController.js | 12 ++- .../web/frontend/extracted-translations.json | 2 + .../share-project-modal/components/invite.jsx | 25 ++--- .../restricted-link-sharing/invite.jsx | 28 +++-- .../share-project-modal-content.tsx | 6 ++ .../share-project-modal-content.tsx | 6 ++ services/web/locales/en.json | 2 + .../CollaboratorsInviteControllerTests.js | 100 ++++++++++++------ 8 files changed, 122 insertions(+), 59 deletions(-) diff --git a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js index 9f9b154a6b..224200ac12 100644 --- a/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js +++ b/services/web/app/src/Features/Collaborators/CollaboratorsInviteController.js @@ -240,6 +240,12 @@ const CollaboratorsInviteController = { inviteId ) + EditorRealTimeController.emitToRoom( + projectId, + 'project:membership:changed', + { invites: true } + ) + if (invite != null) { ProjectAuditLogHandler.addEntryInBackground( projectId, @@ -251,9 +257,11 @@ const CollaboratorsInviteController = { privileges: invite.privileges, } ) - } - res.status(201).json({ newInviteId: invite._id }) + res.sendStatus(201) + } else { + res.sendStatus(404) + } }, async viewInvite(req, res) { diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 3d505b16fc..6ed5bfa7a6 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -657,8 +657,10 @@ "invalid_password_too_similar": "", "invalid_request": "", "invite": "", + "invite_expired": "", "invite_more_collabs": "", "invite_not_accepted": "", + "invite_resend_limit_hit": "", "invited_to_group": "", "invited_to_group_have_individual_subcription": "", "invited_to_join": "", diff --git a/services/web/frontend/js/features/share-project-modal/components/invite.jsx b/services/web/frontend/js/features/share-project-modal/components/invite.jsx index c44f712564..73f844a67b 100644 --- a/services/web/frontend/js/features/share-project-modal/components/invite.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/invite.jsx @@ -44,25 +44,21 @@ Invite.propTypes = { function ResendInvite({ invite }) { const { t } = useTranslation() - const { updateProject, monitorRequest } = useShareProjectContext() - const { _id: projectId, invites } = useProjectContext() + const { monitorRequest, setError, inFlight } = useShareProjectContext() + const { _id: projectId } = useProjectContext() // const buttonRef = useRef(null) // const handleClick = useCallback( () => monitorRequest(() => resendInvite(projectId, invite)) - .then(({ newInviteId }) => { - const updatedInvites = invites.map(existing => { - if (existing === invite) { - // Update the invitation id for the project - existing._id = newInviteId - } - return existing - }) - updateProject({ - invites: updatedInvites, - }) + .catch(error => { + if (error?.response?.status === 404) { + setError('invite_expired') + } + if (error?.response?.status === 429) { + setError('invite_resend_limit_hit') + } }) .finally(() => { // NOTE: disabled as react-bootstrap v0.33.1 isn't forwarding the ref to the `button` @@ -71,7 +67,7 @@ function ResendInvite({ invite }) { // } document.activeElement.blur() }), - [invite, monitorRequest, projectId, invites, updateProject] + [invite, monitorRequest, projectId, setError] ) return ( @@ -79,6 +75,7 @@ function ResendInvite({ invite }) { bsStyle="link" className="btn-inline-link" onClick={handleClick} + disabled={inFlight} // ref={buttonRef} > {t('resend')} diff --git a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/invite.jsx b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/invite.jsx index 63be4e39db..e6f1572d35 100644 --- a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/invite.jsx +++ b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/invite.jsx @@ -43,21 +43,30 @@ Invite.propTypes = { function ResendInvite({ invite }) { const { t } = useTranslation() - const { monitorRequest } = useShareProjectContext() + const { monitorRequest, setError, inFlight } = useShareProjectContext() const { _id: projectId } = useProjectContext() // const buttonRef = useRef(null) // const handleClick = useCallback( () => - monitorRequest(() => resendInvite(projectId, invite)).finally(() => { - // NOTE: disabled as react-bootstrap v0.33.1 isn't forwarding the ref to the `button` - // if (buttonRef.current) { - // buttonRef.current.blur() - // } - document.activeElement.blur() - }), - [invite, monitorRequest, projectId] + monitorRequest(() => resendInvite(projectId, invite)) + .catch(error => { + if (error?.response?.status === 404) { + setError('invite_expired') + } + if (error?.response?.status === 429) { + setError('invite_resend_limit_hit') + } + }) + .finally(() => { + // NOTE: disabled as react-bootstrap v0.33.1 isn't forwarding the ref to the `button` + // if (buttonRef.current) { + // buttonRef.current.blur() + // } + document.activeElement.blur() + }), + [invite, monitorRequest, projectId, setError] ) return ( @@ -65,6 +74,7 @@ function ResendInvite({ invite }) { bsStyle="link" className="btn-inline-link" onClick={handleClick} + disabled={inFlight} // ref={buttonRef} > {t('resend')} diff --git a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/share-project-modal-content.tsx b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/share-project-modal-content.tsx index 44b43915af..8c464b8952 100644 --- a/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/share-project-modal-content.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/restricted-link-sharing/share-project-modal-content.tsx @@ -98,6 +98,12 @@ function ErrorMessage({ error }: Pick) { case 'too_many_requests': return <>{t('too_many_requests')} + case 'invite_expired': + return <>{t('invite_expired')} + + case 'invite_resend_limit_hit': + return <>{t('invite_resend_limit_hit')} + default: return <>{t('generic_something_went_wrong')} } diff --git a/services/web/frontend/js/features/share-project-modal/components/share-project-modal-content.tsx b/services/web/frontend/js/features/share-project-modal/components/share-project-modal-content.tsx index ec89b2dd1f..b63c95b73c 100644 --- a/services/web/frontend/js/features/share-project-modal/components/share-project-modal-content.tsx +++ b/services/web/frontend/js/features/share-project-modal/components/share-project-modal-content.tsx @@ -98,6 +98,12 @@ function ErrorMessage({ error }: Pick) { case 'too_many_requests': return <>{t('too_many_requests')} + case 'invite_expired': + return <>{t('invite_expired')} + + case 'invite_resend_limit_hit': + return <>{t('invite_resend_limit_hit')} + default: return <>{t('generic_something_went_wrong')} } diff --git a/services/web/locales/en.json b/services/web/locales/en.json index e15aee45b0..7667c51bfc 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -951,10 +951,12 @@ "invalid_request": "Invalid Request. Please correct the data and try again.", "invalid_zip_file": "Invalid zip file", "invite": "Invite", + "invite_expired": "The invite may have expired", "invite_more_collabs": "Invite more collaborators", "invite_not_accepted": "Invite not yet accepted", "invite_not_valid": "This is not a valid project invite", "invite_not_valid_description": "The invite may have expired. Please contact the project owner", + "invite_resend_limit_hit": "The invite resend limit hit", "invited_to_group": "<0>__inviterName__ has invited you to join a group subscription on __appName__", "invited_to_group_have_individual_subcription": "__inviterName__ has invited you to join a group __appName__ subscription. If you join this group, you may not need your individual subscription. Would you like to cancel it?", "invited_to_group_login": "To accept this invitation you need to log in as __emailAddress__.", diff --git a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js index da07b111b4..cfa49b9c75 100644 --- a/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js +++ b/services/web/test/unit/src/Collaborators/CollaboratorsInviteControllerTests.js @@ -1269,46 +1269,78 @@ describe('CollaboratorsInviteController', function () { }) describe('when generateNewInvite does not produce an error', function () { - beforeEach(function (done) { - this.res.callback = () => done() - this.CollaboratorsInviteController.generateNewInvite( - this.req, - this.res, - this.next - ) - }) + describe('and returns an invite object', function () { + beforeEach(function (done) { + this.res.callback = () => done() + this.CollaboratorsInviteController.generateNewInvite( + this.req, + this.res, + this.next + ) + }) - it('should produce a 201 response with new invitation id', function () { - this.res.status.callCount.should.equal(1) - this.res.status.calledWith(201).should.equal(true) - expect(this.res.json.firstCall.args[0]).to.deep.equal({ - newInviteId: this.invite._id, + it('should produce a 201 response', function () { + this.res.sendStatus.callCount.should.equal(1) + this.res.sendStatus.calledWith(201).should.equal(true) + }) + + it('should have called generateNewInvite', function () { + this.CollaboratorsInviteHandler.promises.generateNewInvite.callCount.should.equal( + 1 + ) + }) + + it('should have called emitToRoom', function () { + this.EditorRealTimeController.emitToRoom.callCount.should.equal(1) + this.EditorRealTimeController.emitToRoom + .calledWith(this.projectId, 'project:membership:changed') + .should.equal(true) + }) + + it('should check the rate limit', function () { + this.CollaboratorsInviteController.promises._checkRateLimit.callCount.should.equal( + 1 + ) + }) + + it('should add a project audit log entry', function () { + this.ProjectAuditLogHandler.addEntryInBackground.should.have.been.calledWith( + this.projectId, + 'resend-invite', + this.currentUser._id, + this.req.ip, + { + inviteId: this.invite._id, + privileges: this.privileges, + } + ) }) }) - it('should have called generateNewInvite', function () { - this.CollaboratorsInviteHandler.promises.generateNewInvite.callCount.should.equal( - 1 - ) - }) + describe('and returns a null invite', function () { + beforeEach(function (done) { + this.CollaboratorsInviteHandler.promises.generateNewInvite.resolves( + null + ) + this.res.callback = () => done() + this.CollaboratorsInviteController.generateNewInvite( + this.req, + this.res, + this.next + ) + }) - it('should check the rate limit', function () { - this.CollaboratorsInviteController.promises._checkRateLimit.callCount.should.equal( - 1 - ) - }) + it('should have called emitToRoom', function () { + this.EditorRealTimeController.emitToRoom.callCount.should.equal(1) + this.EditorRealTimeController.emitToRoom + .calledWith(this.projectId, 'project:membership:changed') + .should.equal(true) + }) - it('should add a project audit log entry', function () { - this.ProjectAuditLogHandler.addEntryInBackground.should.have.been.calledWith( - this.projectId, - 'resend-invite', - this.currentUser._id, - this.req.ip, - { - inviteId: this.invite._id, - privileges: this.privileges, - } - ) + it('should produce a 404 response when invite is null', function () { + this.res.sendStatus.callCount.should.equal(1) + this.res.sendStatus.should.have.been.calledWith(404) + }) }) })