Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor GitLab integration #1116

Merged
merged 29 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
88a5619
Add Gtilab functionalities
alessandrozago Jul 31, 2024
8672ba9
Fix new gitlab files using eslint
alessandrozago Aug 12, 2024
5dbe093
Add tests for GitLab class and format fixes
alessandrozago Sep 6, 2024
b67e361
Remove existing GitLab repository in the config
alessandrozago Sep 12, 2024
46567ed
Add EUPL-1.2 copyright
alessandrozago Oct 8, 2024
3a64bf0
Use only a single template for README
alessandrozago Oct 9, 2024
d46c68c
Fix release zip upload not working
alessandrozago Oct 21, 2024
e423dd3
Update changelog
alessandrozago Oct 21, 2024
61b20a6
Merge branch 'main' into main
alessandrozago Oct 22, 2024
3b964ca
Fix typo on changelog
alessandrozago Oct 22, 2024
07aac6f
Remove explicit EUPL-1.2 copyright in files
alessandrozago Oct 29, 2024
bbf5e58
Align logging messages format with latest releases
alessandrozago Oct 29, 2024
00a3076
Merge branch 'main' into main
alessandrozago Oct 29, 2024
bb72636
Merge branch 'main' into main
alessandrozago Nov 5, 2024
3b92517
Factorize code between Gitlab and GitHub reporters
Ndpnt Nov 5, 2024
48ad093
Factorize code between Gitlab and GitHub publisher
Ndpnt Nov 5, 2024
ca58f97
Fix deprecated method
Ndpnt Nov 5, 2024
a6d8d29
Improve changelog
Ndpnt Nov 5, 2024
64d1060
Improve changelog entry
Ndpnt Nov 7, 2024
8769d0f
Add release funders
Ndpnt Nov 7, 2024
aed17f3
Clarify token precedence between GitHub and GitLab
Ndpnt Nov 7, 2024
cd411eb
Update changelog entry
Ndpnt Nov 12, 2024
c9fa05e
Add backward compatibility for legacy config
Ndpnt Nov 12, 2024
4496f52
Use proper configuration key
Ndpnt Nov 12, 2024
24875eb
Improve test maintainability
Ndpnt Nov 13, 2024
0bdd637
Improve comment
Ndpnt Nov 13, 2024
356a8b8
Implement no-op clearCache in GitLab reporter
Ndpnt Nov 13, 2024
2dda758
Remove obsolete log
Ndpnt Nov 13, 2024
7df61b6
Improve wording
Ndpnt Nov 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,13 @@ export default async function track({ services, types, extractOnly, schedule })
}

if (process.env.OTA_ENGINE_GITHUB_TOKEN || process.env.OTA_ENGINE_GITLAB_TOKEN) {
if (config.has('@opentermsarchive/engine.reporter.repositories.declarations')) {
try {
const reporter = new Reporter(config.get('@opentermsarchive/engine.reporter'));

await reporter.initialize();
archivist.attach(reporter);
} catch (error) {
logger.error('Cannot instantiate the Reporter module; it will be ignored:', error);
}
} else {
logger.warn('Configuration key "reporter.repositories.declarations" was not found; issues on the declarations repository cannot be created');
try {
const reporter = new Reporter(config.get('@opentermsarchive/engine.reporter'));

await reporter.initialize();
archivist.attach(reporter);
} catch (error) {
logger.error('Cannot instantiate the Reporter module; it will be ignored:', error);
}
} else {
logger.warn('Environment variable with token for GitHub or GitLab was not found; the Reporter module will be ignored');
Expand Down
52 changes: 45 additions & 7 deletions src/reporter/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import mime from 'mime';

import { toISODateWithoutMilliseconds } from '../archivist/utils/date.js';
import logger from '../logger/index.js';

import { createReporter } from './factory.js';

Expand Down Expand Up @@ -33,16 +34,53 @@ function getLabelNameFromError(error) {
// In the following class, it is assumed that each issue is managed using its title as a unique identifier
export default class Reporter {
constructor(config) {
const { repositories } = config;
const normalizedConfig = Reporter.normalizeConfig(config);

for (const repositoryType of Object.keys(repositories)) {
if (!repositories[repositoryType].includes('/') || repositories[repositoryType].includes('https://')) {
throw new Error(`Configuration entry "reporter.repositories.${repositoryType}" is expected to be a string in the format <owner>/<repo>, but received: "${repositories[repositoryType]}"`);
}
Reporter.validateConfiguration(normalizedConfig.repositories);

this.reporter = createReporter(normalizedConfig);
this.repositories = normalizedConfig.repositories;
}

/**
* Support for legacy config format where reporter configuration was nested under "githubIssues"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Support for legacy config format where reporter configuration was nested under "githubIssues"
* Support for legacy config format where reporter configuration was nested under `githubIssues`

* Example:
*
* ```json
* {
* "githubIssues": {
* "repositories": {
* "declarations": "OpenTermsArchive/sandbox-declarations"
* }
* }
* }
* ```
*
* @deprecated
*/
static normalizeConfig(config) {
if (config.githubIssues) {
logger.warn('The "reporter.githubIssues" key is deprecated; please see configuration documentation for the new format: https://docs.opentermsarchive.org/#configuring');

return {
type: 'github',
repositories: config.githubIssues.repositories,
};
}

return config;
}

static validateConfiguration(repositories) {
if (!repositories?.declarations) {
throw new Error('Required configuration key "reporter.repositories.declarations" was not found; issues on the declarations repository cannot be created');
}

this.reporter = createReporter(config);
this.repositories = repositories;
for (const [ type, repo ] of Object.entries(repositories)) {
if (!repo.includes('/') || repo.includes('https://')) {
throw new Error(`Configuration entry "reporter.repositories.${type}" is expected to be a string in the format <owner>/<repo>, but received: "${repo}"`);
}
}
}

initialize() {
Expand Down
63 changes: 63 additions & 0 deletions src/reporter/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { expect } from 'chai';

import Reporter from './index.js';

describe('Reporter', () => {
describe('#normalizeConfig', () => {
context('with current config format', () => {
it('returns the config as is', () => {
const config = { repositories: { declarations: 'owner/repo' } };
const normalizedConfig = Reporter.normalizeConfig(config);

expect(normalizedConfig).to.deep.equal(config);
});
});

context('with old config format where githubIssues is nested under reporter', () => {
it('returns a normalized config', () => {
const config = { githubIssues: { repositories: { declarations: 'owner/repo' } } };
const expectedConfig = {
type: 'github',
repositories: { declarations: 'owner/repo' },
};
const normalizedConfig = Reporter.normalizeConfig(config);

expect(normalizedConfig).to.deep.equal(expectedConfig);
});
});
});

describe('#validateConfiguration', () => {
context('with valid configuration', () => {
it('does not throw an error', () => {
const repositories = { declarations: 'owner/repo' };

expect(() => {
Reporter.validateConfiguration(repositories);
}).not.to.throw();
});
});

context('with invalid configuration', () => {
context('when declarations key is missing', () => {
it('throws an error', () => {
const repositories = {};

expect(() => {
Reporter.validateConfiguration(repositories);
}).to.throw('Required configuration key "reporter.repositories.declarations" was not found; issues on the declarations repository cannot be created');
Copy link
Member

Choose a reason for hiding this comment

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

I recommend checking against just a significant subset of the string rather than the entire error message, to maximise tests maintainability.

});
});

context('when repository format is incorrect', () => {
it('throws an error', () => {
const repositories = { declarations: 'invalidFormat' };

expect(() => {
Reporter.validateConfiguration(repositories);
}).to.throw('Configuration entry "reporter.repositories.declarations" is expected to be a string in the format <owner>/<repo>, but received: "invalidFormat"');
});
});
});
});
});
Loading