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

feat: Add discussionUrl rules checking against GH issues #445

Merged
merged 11 commits into from
Mar 11, 2024
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ this.targetValidationModes = ['C1Request', 'C2Response'];

Set `this.meta` to explain what the rule is testing for.

Defining this detail here makes it easier for libaries scrape all of the rules that the validator will run.
Defining this detail here makes it easier for libaries to scrape all of the rules that the validator will run.

This meta object should include:

Expand Down
27 changes: 27 additions & 0 deletions src/helpers/self-indexing-object.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* A shorthand to create an object where keys and values are the same.
*
* e.g.
*
* ```js
* > SelfIndexingObject.create(['a', 'b', 'c'])
* { a: 'a', b: 'b', c: 'c' }
* ```
*/
const SelfIndexingObject = {
/**
* @template TKey
* @param {TKey[]} keys
* @returns {Record<TKey, TKey>}
*/
create(keys) {
return keys.reduce((acc, key) => {
acc[key] = key;
return acc;
}, /** @type {any} */({}));
},
};

module.exports = {
SelfIndexingObject,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const DiscussionUrlShouldPointToRecognisedDiscussionBoardRule = require('./discussion-url-should-point-to-recognised-discussion-board-rule');
const ModelNode = require('../../classes/model-node');
const Model = require('../../classes/model');
const DataModelHelper = require('../../helpers/data-model');
const ValidationErrorType = require('../../errors/validation-error-type');
const ValidationErrorCategory = require('../../errors/validation-error-category');
const ValidationErrorSeverity = require('../../errors/validation-error-severity');

describe('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule', () => {
const rule = new DiscussionUrlShouldPointToRecognisedDiscussionBoardRule();

const model = new Model(
DataModelHelper.loadModel('Dataset', 'latest'),
'latest',
true,
);

it('should target Dataset discussionUrl', () => {
const isTargeted = rule.isFieldTargeted(model, 'discussionUrl');
expect(isTargeted).toBe(true);
});

it('should return no error when discussionUrl points to a GitHub repo\'s /issues', async () => {
const nodeToTest = new ModelNode('$', {
discussionUrl: 'https://github.com/openactive/openactive-test-suite/issues',
}, null, model);
const errors = await rule.validate(nodeToTest);
expect(errors).toHaveSize(0);
});

it('should return an error when discussionUrl points to a GitHub repo but not to /issues', async () => {
const nodeToTest = new ModelNode('$', {
discussionUrl: 'https://github.com/openactive/openactive-test-suite',
}, null, model);
const errors = await rule.validate(nodeToTest);
expect(errors).toHaveSize(1);
expect(errors[0].rule).toEqual('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule');
expect(errors[0].category).toEqual(ValidationErrorCategory.DATA_QUALITY);
expect(errors[0].type).toEqual(ValidationErrorType.INVALID_FORMAT);
expect(errors[0].severity).toEqual(ValidationErrorSeverity.FAILURE);
});

it('should return a warning when discussionUrl points to an unrecognised place', async () => {
const nodeToTest = new ModelNode('$', {
discussionUrl: 'https://example.com/some-place',
}, null, model);
const errors = await rule.validate(nodeToTest);
expect(errors).toHaveSize(1);
expect(errors[0].rule).toEqual('DiscussionUrlShouldPointToRecognisedDiscussionBoardRule');
expect(errors[0].category).toEqual(ValidationErrorCategory.CONFORMANCE);
expect(errors[0].type).toEqual(ValidationErrorType.INVALID_FORMAT);
expect(errors[0].severity).toEqual(ValidationErrorSeverity.WARNING);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
const Rule = require('../rule');
const ValidationErrorType = require('../../errors/validation-error-type');
const ValidationErrorCategory = require('../../errors/validation-error-category');
const ValidationErrorSeverity = require('../../errors/validation-error-severity');
const { SelfIndexingObject } = require('../../helpers/self-indexing-object');

const TEST_KEYS = SelfIndexingObject.create(/** @type {const} */([
'githubButNotIssues',
'unrecognisedFormat',
]));

module.exports = class DiscussionUrlShouldPointToRecognisedDiscussionBoardRule extends Rule {
constructor(options) {
super(options);
this.targetFields = {
Dataset: ['discussionUrl'],
};
this.meta = {
name: 'DiscussionUrlShouldPointToRecognisedDiscussionBoardRule',
description: 'Validates that the `discussionUrl` property points to a recognised discussion board.',
tests: {
[TEST_KEYS.githubButNotIssues]: {
message: 'If `discussionUrl` points to GitHub, it should be to the project\'s /issues page e.g. `https://github.com/openactive/OpenActive.Server.NET/issues`.',
category: ValidationErrorCategory.DATA_QUALITY,
severity: ValidationErrorSeverity.FAILURE,
type: ValidationErrorType.INVALID_FORMAT,
},
[TEST_KEYS.unrecognisedFormat]: {
message: 'The `discussionUrl` property does not point to a recognised discussion board. Currently recognised discussion board formats: `https://github.com/<ORG>/<REPO>/issues`.',
category: ValidationErrorCategory.CONFORMANCE,
severity: ValidationErrorSeverity.WARNING,
type: ValidationErrorType.INVALID_FORMAT,
},
},
};
}

/**
* @param {import('../../classes/model-node').ModelNodeType} node
* @param {string} field
*/
async validateField(node, field) {
const discussionUrlRaw = node.getValue(field);
const discussionUrl = new URL(discussionUrlRaw);
if (discussionUrl.hostname === 'github.com') {
return this.validateGitHubUrl(discussionUrl, node, field);
}
return [
this.createError(
TEST_KEYS.unrecognisedFormat,
{
value: node.getValue(field),
path: node.getPath(field),
},
),
];
}

/**
* @param {URL} discussionUrl
* @param {import('../../classes/model-node').ModelNodeType} node
* @param {string} field
*/
validateGitHubUrl(discussionUrl, node, field) {
const isGhIssuesUrl = discussionUrl.pathname.endsWith('/issues');
if (isGhIssuesUrl) {
return [];
}
return [
this.createError(
TEST_KEYS.githubButNotIssues,
{
value: node.getValue(field),
path: node.getPath(field),
},
),
];
}
};
4 changes: 3 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
"include": [
"src/classes/**/*.js",
"src/errors/**/*.js",
"src/exceptions.js",
"src/helpers/**/*.js",
"src/rules/rule.js",
"src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule.js",
"src/rules/data-quality/discussion-url-should-point-to-recognised-discussion-board-rule-spec.js",
// TODO add other rules
"src/exceptions.js"
],
"exclude": [
"src/helpers/graph.js"
Expand Down
Loading