Skip to content

Commit 9f4b569

Browse files
sadiqkhojasadiqkhoja
committed
Incorporated PR Feedback
1 parent 84f23db commit 9f4b569

6 files changed

Lines changed: 72 additions & 55 deletions

File tree

‎docs/api.yaml‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ info:
4444
## ODK Central v2026.2
4545

4646
**Changed**:
47-
- The [Getting Form Details](/central-api-form-management/#getting-form-details), [Listing Published Form Versions](/central-api-form-management/#listing-published-form-versions) and [Getting Form Version Details](/central-api-form-management/#getting-form-version-details) endpoints now return a `publishingNotes` field (conditionally) in extended metadata.
47+
- The [Getting Form Details](/central-api-form-management/#getting-form-details), [Listing Published Form Versions](/central-api-form-management/#listing-published-form-versions) and [Getting Form Version Details](/central-api-form-management/#getting-form-version-details) endpoints now return a `publishNotes` field in extended metadata.
4848

4949
## ODK Central v2026.1
5050

@@ -4618,7 +4618,7 @@ paths:
46184618
type: user
46194619
updatedAt: 2018-04-18T23:42:11.406Z
46204620
deletedAt: 2018-04-18T23:42:11.406Z
4621-
publishingNotes: Fixed validation rules for required fields
4621+
publishNotes: Fixed validation rules for required fields
46224622
/v1/projects/{projectId}/forms/{xmlFormId}/versions/{version}:
46234623
get:
46244624
tags:
@@ -12618,9 +12618,10 @@ components:
1261812618
type: number
1261912619
example: 4
1262012620
description: The number of Public Links that can submit to the Form. This does not include Public Links that have been revoked.
12621-
publishingNotes:
12621+
publishNotes:
1262212622
type: string
12623-
description: The notes provided when this version was published (via the `X-Action-Notes` header). Only returned to users with `form.update` permission.
12623+
example: Fixed validation rules for required fields
12624+
description: The notes provided when the current Form version was published (via the `X-Action-Notes` header). Only returned to users with `form.update` permission.
1262412625
ExtendedFormVersion:
1262512626
allOf:
1262612627
- $ref: '#/components/schemas/Form'
@@ -12632,8 +12633,9 @@ components:
1263212633
excelContentType:
1263312634
type: string
1263412635
description: If the Form was created by uploading an Excel file, this field contains the MIME type of that file.
12635-
publishingNotes:
12636+
publishNotes:
1263612637
type: string
12638+
example: Fixed validation rules for required fields
1263712639
description: The notes provided when this version was published (via the `X-Action-Notes` header). Only returned to users with `form.update` permission.
1263812640
FormAttachment:
1263912641
type: object

‎lib/model/frames/form.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ Form.Partial = class extends Form {};
161161

162162
Form.Extended = class extends Frame.define(
163163
'submissions', readable, 'lastSubmission', readable,
164-
'excelContentType', readable, 'publishingNotes', readable,
164+
'excelContentType', readable, 'publishNotes', readable,
165165
// counts of submissions in various review states
166166
'receivedCount', 'hasIssuesCount',
167167
'editedCount', 'entityRelated', readable,
@@ -178,15 +178,15 @@ Form.Extended = class extends Frame.define(
178178
},
179179
lastSubmission: this.lastSubmission,
180180
excelContentType: this.excelContentType,
181-
publishingNotes: this.publishingNotes,
181+
publishNotes: this.publishNotes,
182182
publicLinks: this.publicLinks ?? 0
183183
};
184184
}
185185
};
186186

187187
Form.ExtendedVersion = Frame.define(
188188
'excelContentType', readable,
189-
'publishingNotes', readable
189+
'publishNotes', readable
190190
);
191191

192192

‎lib/model/query/forms.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,7 @@ select ${fields} from forms
608608
join form_defs on ${versionJoinCondition(Form.AllVersions)}
609609
${extend|| sql`
610610
left outer join (
611-
select distinct on ("acteeId", details->'newDefId') "acteeId", "actorId", details, notes as "publishingNotes"
611+
select distinct on ("acteeId", details->'newDefId') "acteeId", "actorId", details, notes as "publishNotes"
612612
from audits where action='form.update.publish'
613613
order by "acteeId", details->'newDefId', "loggedAt" desc
614614
) as audits
@@ -672,7 +672,7 @@ ${extend|| sql`
672672
left outer join (select * from audits where action='form.create') as audits
673673
on forms."acteeId"=audits."acteeId"
674674
left outer join (
675-
select distinct on ("acteeId", details->'newDefId') "acteeId", details, notes as "publishingNotes"
675+
select distinct on ("acteeId", details->'newDefId') "acteeId", details, notes as "publishNotes"
676676
from audits where action='form.update.publish'
677677
order by "acteeId", details->'newDefId', "loggedAt" desc
678678
) as publish_audits

‎lib/resources/forms.js‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,25 @@ const excelMimeTypes = {
2929
xlsx: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'
3030
};
3131

32-
const _checkActorPermission = (authOrVerbs, requiredVerbs, form) => {
33-
if (authOrVerbs instanceof Array) {
34-
const requiredVerbsArray = typeof requiredVerbs === 'string' ? [requiredVerbs] : requiredVerbs;
35-
const authOrVerbsSet = new Set(authOrVerbs);
36-
if (requiredVerbsArray.every(v => !authOrVerbsSet.has(v))) {
37-
return reject(Problem.user.insufficientRights());
38-
}
39-
return form;
40-
} else {
41-
return authOrVerbs.canOrReject(requiredVerbs, form);
32+
const checkActorVerbs = (actorVerbs, requiredVerbs) => {
33+
const actorVerbsSet = new Set(actorVerbs);
34+
if (requiredVerbs.every(v => !actorVerbsSet.has(v))) {
35+
return reject(Problem.user.insufficientRights());
4236
}
4337
};
4438

45-
const canReadForm = (authOrVerbs, form) => {
39+
const canReadForm = async (authOrVerbs, form) => {
40+
const verbs = authOrVerbs instanceof Array
41+
? authOrVerbs
42+
: (await authOrVerbs.verbsOn(form));
4643
if (form.def?.publishedAt == null) {
47-
return _checkActorPermission(authOrVerbs, 'form.update', form);
44+
await checkActorVerbs(verbs, ['form.update']);
4845
} else if (form.state === 'closed') {
49-
return _checkActorPermission(authOrVerbs, 'form.read', form);
46+
await checkActorVerbs(verbs, ['form.read']);
5047
} else {
51-
return _checkActorPermission(authOrVerbs, ['open_form.read', 'form.read'], form);
48+
await checkActorVerbs(verbs, ['open_form.read', 'form.read']);
5249
}
50+
return form;
5351
};
5452
// Returns QueryOptions to use to limit entity access according to ownerOnly.
5553
const getOwnerOnlyOptions = async (auth, project) => ((await auth.can('entity.list', project))
@@ -296,7 +294,7 @@ module.exports = (service, endpoint, anonymousEndpoint) => {
296294

297295
await canReadForm(verbs, form);
298296

299-
return verbs.includes('form.update') ? form : omit(['publishingNotes'], form.forApi());
297+
return verbs.includes('form.update') ? form : omit(['publishNotes'], form.forApi());
300298
}));
301299

302300
// returns form fields, optionally sanitizing names to match odata.
@@ -394,7 +392,7 @@ module.exports = (service, endpoint, anonymousEndpoint) => {
394392

395393
const versions = await Forms.getVersions(form.id, queryOptions);
396394

397-
return verbs.includes('form.update') ? versions : versions.map(v => omit(['publishingNotes'], v.forApi()));
395+
return verbs.includes('form.update') ? versions : versions.map(v => omit(['publishNotes'], v.forApi()));
398396
}));
399397

400398
////////////////////////////////////////

‎test/integration/api/forms/forms.js‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,8 +1094,8 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
10941094
}));
10951095
}))));
10961096

1097-
describe('publishingNotes', () => {
1098-
it('should return publishingNotes with extended metadata', testService(async (service) => {
1097+
describe('publishNotes', () => {
1098+
it('should return publishNotes with extended metadata', testService(async (service) => {
10991099
const asAlice = await service.login('alice');
11001100

11011101
await publishWithNote(asAlice, '2', 'this is a publishing note');
@@ -1104,10 +1104,10 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
11041104
.set('X-Extended-Metadata', true)
11051105
.expect(200);
11061106

1107-
body.publishingNotes.should.equal('this is a publishing note');
1107+
body.publishNotes.should.equal('this is a publishing note');
11081108
}));
11091109

1110-
it('should not return publishingNotes for app-user', testService(async (service) => {
1110+
it('should not return publishNotes for app-user', testService(async (service) => {
11111111
const asAlice = await service.login('alice');
11121112

11131113
await publishWithNote(asAlice, '2', 'this is a publishing note');
@@ -1123,10 +1123,10 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
11231123
.set('X-Extended-Metadata', true)
11241124
.expect(200);
11251125

1126-
should.not.exist(body.publishingNotes);
1126+
should.not.exist(body.publishNotes);
11271127
}));
11281128

1129-
it('should not return publishingNotes for project-viewer', testService(async (service) => {
1129+
it('should not return publishNotes for project-viewer', testService(async (service) => {
11301130
const asAlice = await service.login('alice');
11311131

11321132
await publishWithNote(asAlice, '2', 'this is a publishing note');
@@ -1142,10 +1142,10 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
11421142
.set('X-Extended-Metadata', true)
11431143
.expect(200);
11441144

1145-
should.not.exist(body.publishingNotes);
1145+
should.not.exist(body.publishNotes);
11461146
}));
11471147

1148-
it('should not return publishingNotes for data-collector', testService(async (service) => {
1148+
it('should not return publishNotes for data-collector', testService(async (service) => {
11491149
const asAlice = await service.login('alice');
11501150

11511151
await publishWithNote(asAlice, '2', 'this is a publishing note');
@@ -1161,10 +1161,10 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
11611161
.set('X-Extended-Metadata', true)
11621162
.expect(200);
11631163

1164-
should.not.exist(body.publishingNotes);
1164+
should.not.exist(body.publishNotes);
11651165
}));
11661166

1167-
it('should return publishingNotes with extended metadata even if draft is published twice', testService(async (service) => {
1167+
it('should return publishNotes with extended metadata even if draft is published twice', testService(async (service) => {
11681168
const asAlice = await service.login('alice');
11691169

11701170
await asAlice.post('/v1/projects/1/forms/simple/draft')
@@ -1194,7 +1194,7 @@ describe('api: /projects/:id/forms (create, read, update)', () => {
11941194
.set('X-Extended-Metadata', true)
11951195
.expect(200);
11961196

1197-
body.publishingNotes.should.equal('this is a publishing note');
1197+
body.publishNotes.should.equal('this is a publishing note');
11981198
}));
11991199
});
12001200
});

‎test/integration/api/forms/versions.js‎

Lines changed: 35 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe('api: /projects/:id/forms (versions)', () => {
195195
body.map((version) => version.version).should.eql([ '2', '3', '' ]);
196196
})))));
197197

198-
describe('publishing notes', () => {
198+
describe('publish notes', () => {
199199
it('should return publish notes', testService(async (service) => {
200200
const asAlice = await service.login('alice');
201201

@@ -207,7 +207,7 @@ describe('api: /projects/:id/forms (versions)', () => {
207207
.expect(200);
208208

209209
body.map((form) => form.version).should.eql(['3', '2', '']);
210-
body.map((form) => form.publishingNotes).should.eql([
210+
body.map((form) => form.publishNotes).should.eql([
211211
'publishing version 3',
212212
'publishing version 2',
213213
null
@@ -231,7 +231,7 @@ describe('api: /projects/:id/forms (versions)', () => {
231231
.expect(200);
232232
body.map((form) => form.version).should.eql(['2', '']);
233233
body.forEach((form) => {
234-
should.not.exist(form.publishingNotes);
234+
should.not.exist(form.publishNotes);
235235
});
236236
}));
237237

@@ -249,7 +249,7 @@ describe('api: /projects/:id/forms (versions)', () => {
249249
.expect(200);
250250
body.map((form) => form.version).should.eql(['2', '']);
251251
body.forEach((form) => {
252-
should.not.exist(form.publishingNotes);
252+
should.not.exist(form.publishNotes);
253253
});
254254
}));
255255

@@ -269,7 +269,7 @@ describe('api: /projects/:id/forms (versions)', () => {
269269
.expect(200);
270270
body.map((form) => form.version).should.eql(['2', '']);
271271
body.forEach((form) => {
272-
should.not.exist(form.publishingNotes);
272+
should.not.exist(form.publishNotes);
273273
});
274274
}));
275275

@@ -289,7 +289,7 @@ describe('api: /projects/:id/forms (versions)', () => {
289289
.expect(200);
290290
body.map((form) => form.version).should.eql(['2', '']);
291291
body.forEach((form) => {
292-
should.not.exist(form.publishingNotes);
292+
should.not.exist(form.publishNotes);
293293
});
294294
}));
295295

@@ -312,7 +312,7 @@ describe('api: /projects/:id/forms (versions)', () => {
312312
.expect(200);
313313

314314
body.map((form) => form.version).should.eql(['2', '']);
315-
body[0].publishingNotes.should.equal('publishing version 2');
315+
body[0].publishNotes.should.equal('publishing version 2');
316316
}));
317317
});
318318
});
@@ -506,8 +506,8 @@ describe('api: /projects/:id/forms (versions)', () => {
506506
})))));
507507
});
508508

509-
describe('publishing notes', () => {
510-
it('should return publishing notes', testService(async (service) => {
509+
describe('publish notes', () => {
510+
it('should return publishNotes', testService(async (service) => {
511511
const asAlice = await service.login('alice');
512512

513513
await publishWithNote(asAlice, '2', 'publishing version 2');
@@ -517,10 +517,10 @@ describe('api: /projects/:id/forms (versions)', () => {
517517
.expect(200);
518518

519519
body.version.should.equal('2');
520-
body.publishingNotes.should.equal('publishing version 2');
520+
body.publishNotes.should.equal('publishing version 2');
521521
}));
522522

523-
it('should not return publishing notes for app-user', testService(async (service) => {
523+
it('should not return publishNotes for app-user', testService(async (service) => {
524524
const asAlice = await service.login('alice');
525525

526526
await publishWithNote(asAlice, '2', 'publishing version 2');
@@ -537,10 +537,10 @@ describe('api: /projects/:id/forms (versions)', () => {
537537
.expect(200);
538538

539539
body.version.should.equal('2');
540-
should.not.exist(body.publishingNotes);
540+
should.not.exist(body.publishNotes);
541541
}));
542542

543-
it('should not return publishing notes for project-viewer', testService(async (service) => {
543+
it('should not return publishNotes for project-viewer', testService(async (service) => {
544544
const asAlice = await service.login('alice');
545545

546546
await publishWithNote(asAlice, '2', 'publishing version 2');
@@ -557,10 +557,27 @@ describe('api: /projects/:id/forms (versions)', () => {
557557
.expect(200);
558558

559559
body.version.should.equal('2');
560-
should.not.exist(body.publishingNotes);
560+
should.not.exist(body.publishNotes);
561561
}));
562562

563-
it('should not return publishing notes for data-collector', testService(async (service) => {
563+
it('should not return notes for public-link', testService(async (service) => {
564+
const asAlice = await service.login('alice');
565+
566+
await publishWithNote(asAlice, '2', 'publishing version 2');
567+
568+
const publicLinkToken = await asAlice.post('/v1/projects/1/forms/simple/public-links')
569+
.send({ displayName: 'test public link' })
570+
.expect(200)
571+
.then(({ body }) => body.token);
572+
573+
const { body } = await service.get(`/v1/projects/1/forms/simple/versions/2?st=${publicLinkToken}`)
574+
.set('X-Extended-Metadata', true)
575+
.expect(200);
576+
body.version.should.equal('2');
577+
should.not.exist(body.publishNotes);
578+
}));
579+
580+
it('should not return publishNotes for data-collector', testService(async (service) => {
564581
const asAlice = await service.login('alice');
565582

566583
await publishWithNote(asAlice, '2', 'publishing version 2');
@@ -577,10 +594,10 @@ describe('api: /projects/:id/forms (versions)', () => {
577594
.expect(200);
578595

579596
body.version.should.equal('2');
580-
should.not.exist(body.publishingNotes);
597+
should.not.exist(body.publishNotes);
581598
}));
582599

583-
it('should return publishing notes even if draft is published twice', testService(async (service) => {
600+
it('should return publishNotes even if draft is published twice', testService(async (service) => {
584601
const asAlice = await service.login('alice');
585602

586603
// create a form draft
@@ -601,7 +618,7 @@ describe('api: /projects/:id/forms (versions)', () => {
601618
.expect(200);
602619

603620
body.version.should.equal('2');
604-
body.publishingNotes.should.equal('publishing version 2');
621+
body.publishNotes.should.equal('publishing version 2');
605622
}));
606623
});
607624
});

0 commit comments

Comments
 (0)