Skip to content

Add feed endpoints#1242

Open
clementbiron wants to merge 11 commits intomainfrom
add-feed-endpoints
Open

Add feed endpoints#1242
clementbiron wants to merge 11 commits intomainfrom
add-feed-endpoints

Conversation

@clementbiron
Copy link
Copy Markdown
Member

This pull request adds Atom feed endpoints to the collection API so consumers can subscribe to recent terms updates.
A new /feed endpoint exposes a collection feed, with variants at /feed/:serviceId and /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.

return Promise.all((await this.#getCommits()).map(commit => this.#toDomain(commit, { deferContentLoading: true })));
}

async findRecent(limit, { serviceId, termsType } = {}) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better and more idiomatic to not add this method and to update findAll with limit and offset params

import versionsRepository, { storageConfig } from './versionsRepository.js';

const TAG_AUTHORITY = 'opentermsarchive.org,2026';
const FEED_AUTHOR_NAME = 'OTA-Bot';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use commit author


const TAG_AUTHORITY = 'opentermsarchive.org,2026';
const FEED_AUTHOR_NAME = 'OTA-Bot';
const DEFAULT_LIMIT = 100;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be moved in the default config file

const DEFAULT_LIMIT = 100;

function getFeedLimit() {
if (config.has('@opentermsarchive/engine.collection-api.feed.limit')) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid accessing config from this file directly. Follow dependency injection pattern instead.

}

function classifyRecordType(version) {
if (version.isFirstRecord) { return RECORD_TYPES.firstRecord; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I would have use a switch here. I find it more meaningful

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; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple lines. Update Eslint if needed

const feedId = `tag:${TAG_AUTHORITY}:feed:${collection.metadata?.id}`;

const versions = await versionsRepository.findRecent(getFeedLimit());
const document = buildFeedDocument({ collection, selfHref, feedId, versions, baseUrl });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is called in all exposed endpoints, factorize it. Maybe into sendAtom if it makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants