Add feed endpoints#1242
Open
clementbiron wants to merge 11 commits intomainfrom
Open
Conversation
c81ac32 to
d2ed924
Compare
d3f3652 to
2582940
Compare
Ndpnt
reviewed
Apr 29, 2026
| return Promise.all((await this.#getCommits()).map(commit => this.#toDomain(commit, { deferContentLoading: true }))); | ||
| } | ||
|
|
||
| async findRecent(limit, { serviceId, termsType } = {}) { |
Contributor
There was a problem hiding this comment.
I think it is better and more idiomatic to not add this method and to update findAll with limit and offset params
Ndpnt
reviewed
Apr 29, 2026
| import versionsRepository, { storageConfig } from './versionsRepository.js'; | ||
|
|
||
| const TAG_AUTHORITY = 'opentermsarchive.org,2026'; | ||
| const FEED_AUTHOR_NAME = 'OTA-Bot'; |
Ndpnt
reviewed
Apr 29, 2026
|
|
||
| const TAG_AUTHORITY = 'opentermsarchive.org,2026'; | ||
| const FEED_AUTHOR_NAME = 'OTA-Bot'; | ||
| const DEFAULT_LIMIT = 100; |
Contributor
There was a problem hiding this comment.
Should be moved in the default config file
Ndpnt
reviewed
Apr 29, 2026
| const DEFAULT_LIMIT = 100; | ||
|
|
||
| function getFeedLimit() { | ||
| if (config.has('@opentermsarchive/engine.collection-api.feed.limit')) { |
Contributor
There was a problem hiding this comment.
Avoid accessing config from this file directly. Follow dependency injection pattern instead.
Ndpnt
reviewed
Apr 29, 2026
| } | ||
|
|
||
| function classifyRecordType(version) { | ||
| if (version.isFirstRecord) { return RECORD_TYPES.firstRecord; } |
Contributor
There was a problem hiding this comment.
I think I would have use a switch here. I find it more meaningful
Ndpnt
reviewed
Apr 29, 2026
| function buildEntryTitle(version) { | ||
| let prefix = COMMIT_MESSAGE_PREFIXES.update; | ||
|
|
||
| if (version.isFirstRecord) { prefix = COMMIT_MESSAGE_PREFIXES.startTracking; } else if (version.isTechnicalUpgrade) { prefix = COMMIT_MESSAGE_PREFIXES.technicalUpgrade; } |
Contributor
There was a problem hiding this comment.
Multiple lines. Update Eslint if needed
Ndpnt
reviewed
Apr 29, 2026
| const feedId = `tag:${TAG_AUTHORITY}:feed:${collection.metadata?.id}`; | ||
|
|
||
| const versions = await versionsRepository.findRecent(getFeedLimit()); | ||
| const document = buildFeedDocument({ collection, selfHref, feedId, versions, baseUrl }); |
Contributor
There was a problem hiding this comment.
As this is called in all exposed endpoints, factorize it. Maybe into sendAtom if it makes sense
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request adds Atom feed endpoints to the collection API so consumers can subscribe to recent terms updates.
A new
/feedendpoint exposes a collection feed, with variants at/feed/:serviceIdand/feed/:serviceId/:termsType.Contrary to what was discussed in #1238, each entry's primary alternate link points to the GitHub commit rather than to the version API. While testing the feed in real readers, a link resolving to a JSON document felt unhelpful for human consumers, who expect to land on a readable page. The version API is still exposed alongside as a related link, so programmatic consumers can discover it from the feed.