Skip to content

Commit e5eaacd

Browse files
committed
merged development into branch
2 parents 6e77ea2 + 8c6ec9f commit e5eaacd

5 files changed

Lines changed: 63 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@
108108
### Updated
109109
- Updated `plans` resolver to have pagination for a specified `userId` with optional `search term`, and added a chained resolver for `PlanSearchResult` for `templateOwnerAffiliationName` [#281]
110110
- Updated `PlanSearchResult` model to include `createdById` and `templateOwnerAffiliationName` for the Admin Users page [#281]
111+
- Update local migration to add RO question to default template
111112
- Updated `users` resolver to include `role` and `affiliationId` [#240]
112113
- Added `findByAffiliationId` and `search` to pass in `role` as optional [#240]
113114
- Added `findByUserId` to `Plan` model to find all plans for a given user [#240]
@@ -245,6 +246,7 @@
245246
- Fixed issue with templates not cloning with sections and questions by updating the `addTemplate` mutation to clone from non-versioned template, section and question [#1006]
246247

247248
### Chore
249+
- Addressed security vulnerability in `nodemailer` and `undici` packages, and added debugging to troubleshoot request feedback failure [#285]
248250
- Updated `nodemailer` to `v9.0.1` and `undici` to `v7.28.0` [#240]
249251
- Updated `fast-xml-parser` to `v1.2.0` and `uuid` to `11.1.1` to address vulnerabilities.
250252
- Added `@types/nodemailer` [#189]

data-migrations/local-only/2026-06-22-0923-add-research-output-to-default-tmplt.sql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,40 @@ VALUES (@default_template_id, @section_id, 1, 'Please list all research outputs
2222
SET @question_id := LAST_INSERT_ID();
2323

2424
-- Then generate/publish the new version
25+
SET @prior_versioned_template_id := (SELECT id FROM versionedTemplates WHERE templateId = @default_template_id AND active = 1);
2526
UPDATE versionedTemplates SET active = 0 WHERE templateId = @default_template_id AND active = 1;
2627
INSERT INTO versionedTemplates (templateId, active, version, versionType, versionedById, comment, name, description, ownerId, visibility, bestPractice, isDefault, languageId, created, createdById, modified, modifiedById)
2728
(SELECT id, 1, 'v3', 'PUBLISHED', createdById, 'Added a research output table question', name, description, ownerId, 'PUBLIC', bestPractice, isDefault, languageId, CURDATE(), createdById, CURDATE(), modifiedById
2829
FROM templates WHERE id = @default_template_id);
2930
SET @versioned_template_id := LAST_INSERT_ID();
3031

32+
-- Make sure the new version has all the existing sections
33+
INSERT INTO versionedSections (versionedTemplateId, sectionId, name, introduction, requirements, guidance, displayOrder, bestPractice, created, createdById, modified, modifiedById)
34+
(SELECT @versioned_template_id, sectionId, name, introduction, requirements, guidance, displayOrder, bestPractice, CURDATE(), createdById, CURDATE(), modifiedById
35+
FROM versionedSections WHERE versionedTemplateId = @prior_versioned_template_id);
36+
37+
-- Then add the new research output section
3138
INSERT INTO versionedSections (versionedTemplateId, sectionId, name, introduction, requirements, guidance, displayOrder, bestPractice, created, createdById, modified, modifiedById)
3239
(SELECT @versioned_template_id, id, name, introduction, requirements, guidance, displayOrder, bestPractice, CURDATE(), createdById, CURDATE(), modifiedById
3340
FROM sections WHERE id = @section_id);
3441
SET @versioned_section_id := LAST_INSERT_ID();
3542

43+
-- Then make sure all the existing section tags and questions are copied across to the new version
44+
INSERT INTO versionedSectionTags (versionedSectionId, tagId, created, createdById, modified, modifiedById)
45+
(SELECT newVSS.id, st.tagId, CURDATE(), st.createdById, CURDATE(), st.modifiedById
46+
FROM versionedSections AS vss
47+
INNER JOIN versionedSectionTags AS st ON vss.id = st.versionedSectionId
48+
INNER JOIN versionedSections AS newVSS on newVSS.sectionId = vss.sectionId AND newVSS.versionedTemplateId = @versioned_template_id
49+
WHERE vss.versionedTemplateId = @prior_versioned_template_id);
50+
51+
INSERT INTO versionedQuestions (versionedTemplateId, versionedSectionId, questionId, questionText, json, requirementText, guidanceText, sampleText, required, displayOrder, created, createdById, modified, modifiedById)
52+
(SELECT @versioned_template_id, newVSS.id, vsq.questionId, vsq.questionText, vsq.json, vsq.requirementText, vsq.guidanceText, vsq.sampleText, vsq.required, vsq.displayOrder, CURDATE(), vsq.createdById, CURDATE(), vsq.modifiedById
53+
FROM versionedSections AS vss
54+
INNER JOIN versionedQuestions vsq ON vss.id = vsq.versionedSectionId
55+
INNER JOIN versionedSections AS newVSS on newVSS.sectionId = vss.sectionId AND newVSS.versionedTemplateId = @versioned_template_id
56+
WHERE vss.versionedTemplateId = @prior_versioned_template_id);
57+
58+
-- Finally add the new section tags and question to the research output question
3659
INSERT INTO versionedSectionTags (versionedSectionId, tagId, created, createdById, modified, modifiedById)
3760
(SELECT @versioned_section_id, tagId, CURDATE(), createdById, CURDATE(), modifiedById
3861
FROM sectionTags WHERE sectionId = @section_id);

src/resolvers/feedback.ts

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -153,61 +153,66 @@ export const resolvers: Resolvers = {
153153

154154
try {
155155
if (isAuthorized(context.token)) {
156+
context.logger.info({ planId, userId: context.token.id, affiliationId: context.token.affiliationId }, `${reference}: authorized, starting`);
157+
156158
const plan = await Plan.findById(reference, context, planId);
157159
if (!plan) {
158160
throw NotFoundError(`Plan with ID ${planId} not found`);
159161
}
162+
context.logger.info({ planId, projectId: plan.projectId, versionedTemplateId: plan.versionedTemplateId }, `${reference}: plan found`);
163+
160164
const project = await Project.findById(reference, context, plan.projectId);
161165
if (!project) {
162166
throw NotFoundError(`Project with ID ${plan.projectId} not found`);
163167
}
168+
context.logger.info({ projectId: project.id }, `${reference}: project found`);
164169

165-
// Get existing feedback for the given planId
166-
const existingFeedback = await PlanFeedback.findByPlanId(
167-
reference,
168-
context,
169-
planId,
170-
);
171-
172-
// If there is already an active feedback round, then do not allow creation of a new one
173-
const hasOpenFeedback = existingFeedback.some(
174-
(fb) => fb.completed === null
175-
);
170+
const existingFeedback = await PlanFeedback.findByPlanId(reference, context, planId);
171+
context.logger.info({ existingFeedbackCount: existingFeedback.length }, `${reference}: existing feedback fetched`);
176172

173+
const hasOpenFeedback = existingFeedback.some((fb) => fb.completed === null);
177174
if (hasOpenFeedback) {
178175
throw ForbiddenError(`There is already feedback in progress for plan ${planId}`);
179176
}
180177

181-
//Feedback request can only be made by ADMINs and SUPERADMINs or a collaborator with PRIMARY access
182-
if (await hasPermissionOnProject(context, project, ProjectCollaboratorAccessLevel.PRIMARY)) {
183-
const feedbackComment = new PlanFeedback({
184-
planId,
185-
messageToOrg: messageToOrg ?? '',
186-
requestedById: context.token.id,
187-
requested: getCurrentDate()
188-
});
178+
const hasPrimaryPermission = await hasPermissionOnProject(context, project, ProjectCollaboratorAccessLevel.PRIMARY);
179+
context.logger.info({ hasPrimaryPermission }, `${reference}: permission check`);
189180

181+
if (hasPrimaryPermission) {
190182
const affiliationId = context.token.affiliationId;
191183
if (!affiliationId) {
192184
throw NotFoundError(`Affiliation for user not found`);
193185
}
186+
context.logger.info({ affiliationId }, `${reference}: looking up affiliation`);
194187

195188
const affiliation = await Affiliation.findByURI(reference, context, affiliationId);
189+
context.logger.info(
190+
{ affiliationUri: affiliation?.uri, feedbackEmailCount: affiliation?.feedbackEmails?.length ?? 0 },
191+
`${reference}: affiliation found`
192+
);
196193

197194
if (affiliation.feedbackEmails.length === 0) {
198-
context.logger.warn(prepareObjectForLogs({ affiliationId }), `Affiliation with ID ${affiliationId} has no feedback emails configured, so no notifications will be sent when requesting feedback`);
195+
context.logger.warn({ affiliationId }, `${reference}: no feedback emails configured`);
199196
}
200197

201198
const planURL = `/projects/${project.id}/dmp/${planId}`;
202199
const planOwnerName = [context.token.givenName, context.token.surname].filter(Boolean).join(' ');
203200
const planTitle = plan.title || 'Untitled Plan';
201+
context.logger.info({ planURL, planOwnerName, planTitle }, `${reference}: sending feedback request email`);
204202

205-
// Send emails to the feedback recipients
206203
await sendFeedbackRequestEmail(context, planOwnerName, planURL, planTitle, affiliation.feedbackEmails, messageToOrg ?? '');
204+
context.logger.info(`${reference}: feedback request email sent`);
205+
206+
const feedbackComment = new PlanFeedback({
207+
planId,
208+
messageToOrg: messageToOrg ?? '',
209+
requestedById: context.token.id,
210+
requested: getCurrentDate()
211+
});
207212

208213
const createdFeedback = await feedbackComment.create(context);
214+
context.logger.info({ createdFeedbackId: createdFeedback?.id ?? null }, `${reference}: feedback record created`);
209215

210-
// Notify all org admins of the feedback request
211216
if (createdFeedback?.id) {
212217
await AdminNotification.addNotificationForAffiliation(
213218
reference,
@@ -216,11 +221,16 @@ export const resolvers: Resolvers = {
216221
'FEEDBACK_REQUESTED',
217222
{ planId },
218223
);
224+
context.logger.info({ affiliationUri: affiliation.uri }, `${reference}: admin notification sent`);
219225
}
220226

221227
return createdFeedback;
222228
}
223229
}
230+
context.logger.warn(
231+
{ hasToken: !!context?.token, userId: context?.token?.id },
232+
`${reference}: reached auth fallthrough — user did not pass permission checks`
233+
);
224234
throw context?.token ? ForbiddenError() : AuthenticationError();
225235
} catch (err) {
226236
if (err instanceof GraphQLError) throw err;

src/services/__tests__/emailService.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ describe('sendEmail', () => {
334334
.replace('%{helpUrl}', `${domain}/help`);
335335

336336
expect(sent).toBe(true);
337-
expect(logger.info).toHaveBeenCalledTimes(emails.length);
337+
expect(logger.info).toHaveBeenCalledTimes(emails.length + 2); // accounting for additional info logs for sending and finishing
338338
expect(mockSendEmail).toHaveBeenCalledTimes(emails.length);
339339
for (const email of emails) {
340340
const expectedHtml = baseHtml.replace('%{adminEmail}', email);

src/services/emailService.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,13 @@ export const sendFeedbackRequestEmail = async (
298298
.replace('%{helpDeskEmail}', emailConfig.helpDeskAddress)
299299
.replace('%{helpUrl}', `${domain}/help`);
300300

301+
context.logger.info(`Sending feedback request email to ${collaboratorEmails.length} collaborators for plan "${planTitle}" at URL ${domain}${planURL}`);
302+
context.logger.debug(prepareObjectForLogs({ collaboratorEmails, planOwnerName, planURL, planTitle, feedbackRequestMessage }), `Feedback request email details`);
303+
301304
// Send each feedback email recipient their own email
302305
for (const email of collaboratorEmails) {
303306
const message = baseMessage.replace('%{adminEmail}', email);
307+
context.logger.debug(prepareObjectForLogs({ email, message }), `Sending feedback request email to ${email}`);
304308
await sendEmail(
305309
context,
306310
'FeedbackRequest',
@@ -311,5 +315,6 @@ export const sendFeedbackRequestEmail = async (
311315
message
312316
);
313317
}
318+
context.logger.info(`Finished sending feedback request emails for plan "${planTitle}"`);
314319
return true;
315320
}

0 commit comments

Comments
 (0)