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.

Sign up for GitHub

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

b/2531 fix license resolution service url and missing extent/theme #11

Merged
merged 3 commits into from
Nov 5, 2021
Merged

b/2531 fix license resolution service url and missing extent/theme #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48d161e
b/2531 fix license resolution service url and missing extent/theme
drspacemanphd Nov 5, 2021
a6353d0
b/2531 revert package json changes
drspacemanphd Nov 5, 2021
e2f9c47
b/2531 removed optional
drspacemanphd Nov 5, 2021
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
3 changes: 2 additions & 1 deletion src/dcat-us/_generate-distributions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ const WMS_SERVER = 'WMSServer';
export const DISTRIBUTION_DEPENDENCIES = [
'layer.geometryType',
'server.spatialReference',
'metadata.metadata.distInfo.distTranOps.onLineSrc'
'metadata.metadata.distInfo.distTranOps.onLineSrc',
'url',
];

/*
Expand Down
3 changes: 2 additions & 1 deletion src/dcat-us/base-dataset-template.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ export const baseDatasetTemplate = {
fn: '{{owner}}',
hasEmail: '{{orgContactEmail:optional}}'
},
accessLevel: 'public'
accessLevel: 'public',
spatial: '{{extent}}'
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about the theme property mentioned in the issue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Apparently this comes along with extent

};
19 changes: 9 additions & 10 deletions src/dcat-us/dataset-formatter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ describe('formatDcatDataset', () => {
const expected = {
'@type': 'dcat:Dataset',
identifier: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
license: null,
license: '',
landingPage: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
title: 'DCAT_Test',
description: 'Some Description',
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('formatDcatDataset', () => {
const expected = {
'@type': 'dcat:Dataset',
identifier: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
license: null,
license: '',
landingPage: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
title: 'DCAT_Test',
description: 'Some Description',
Expand Down Expand Up @@ -251,7 +251,7 @@ describe('formatDcatDataset', () => {
const expected = {
'@type': 'dcat:Dataset',
identifier: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
license: null,
license: '',
landingPage: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
title: 'DCAT_Test',
description: 'Some Description',
Expand Down Expand Up @@ -342,7 +342,7 @@ describe('formatDcatDataset', () => {
const expected = {
'@type': 'dcat:Dataset',
identifier: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
license: null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@drspacemanphd I changed license to be null instead of '' for an issue a couple months ago. Are we sure we want to go back?

Copy link
Collaborator

Choose a reason for hiding this comment

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

After reviewing, the switch to null was a style preference on Tate's part, not a product requirement. I'm fine to leave this.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sonofflynn89 @drspacemanphd

It's not really a style thing on my part. It all comes down to the DCAT harvesters. Thomas told me that they seemed to prefer null, so I tried to be consistent with that. If it doesn't matter, this is fine with me!

Another thing to be aware of is that when you have a JSON-LD node, sometimes the entire node should be removed when one of the values isn't available.

E.g.

"foo": {
  "@id": "dct:somekey",
  "importantValue": null
}

I think adlib can do this by means of the "Optional Transform.".

license: '',
landingPage: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
title: 'DCAT_Test',
description: 'Some Description',
Expand Down Expand Up @@ -437,7 +437,7 @@ describe('formatDcatDataset', () => {
const expected = {
'@type': 'dcat:Dataset',
identifier: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
license: null,
license: '',
landingPage: `${siteUrl}/datasets/00000000000000000000000000000000_0`,
title: 'DCAT_Test',
description: 'Some Description',
Expand Down Expand Up @@ -535,7 +535,7 @@ describe('formatDcatDataset', () => {
expect(actual.license).toEqual(expectedLicense);
});

it('license should be empty when structuredLicense.url is unavailable', () => {
it('license should use licenseInfo when structuredLicense.url is unavailable', () => {
const dataset = {
owner: 'fpgis.CALFIRE',
created: 1570747289000,
Expand Down Expand Up @@ -565,13 +565,13 @@ describe('formatDcatDataset', () => {
structuredLicense: { text: 'structuredLicense text' },
licenseInfo: 'licenseInfo text',
};
const expectedLicense = null;
const expectedLicense = 'licenseInfo text';

const actual = JSON.parse(formatDcatDataset(dataset, siteUrl, buildDatasetTemplate()));
expect(actual.license).toEqual(expectedLicense);
});

it('license should display null when structuredLicense is unavailable', () => {
it('license should display empty string when neither structuredLicense nor licenseInfo are available', () => {
const dataset = {
owner: 'fpgis.CALFIRE',
created: 1570747289000,
Expand All @@ -598,9 +598,8 @@ describe('formatDcatDataset', () => {
wkid: 3310,
},
},
licenseInfo: 'licenseInfo text',
};
const expectedLicense = null;
const expectedLicense = '';

const actual = JSON.parse(formatDcatDataset(dataset, siteUrl, buildDatasetTemplate()));
expect(actual.license).toEqual(expectedLicense);
Expand Down
8 changes: 6 additions & 2 deletions src/dcat-us/dataset-formatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@ export type DcatDatasetTemplate = Record<string, any>;
export function formatDcatDataset (hubDataset: HubDatasetAttributes, siteUrl: string, datasetTemplate: DcatDatasetTemplate) {
const landingPage = `${siteUrl}/datasets/${hubDataset.id}`;

const { structuredLicense: { url = null } = {} } = hubDataset;
const {
structuredLicense: { url = null } = {},
licenseInfo = ''
} = hubDataset;

const defaultDataset = {
'@type': 'dcat:Dataset',
identifier: landingPage,
license: url,
license: url || licenseInfo || '',
landingPage
};

Expand Down Expand Up @@ -69,6 +72,7 @@ function scrubProtectedKeys (customizations: DcatDatasetTemplate): DcatDatasetTe

if (Object.keys(scrubbedCustomizations).length > 0) {
delete scrubbedCustomizations['@type'];
delete scrubbedCustomizations.license;
delete scrubbedCustomizations.identifier;
delete scrubbedCustomizations.landingPage;
delete scrubbedCustomizations.webService;
Expand Down
16 changes: 12 additions & 4 deletions src/dcat-us/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('generating DCAT-US 1.1 feed', () => {

expect(chk1['@type']).toBe('dcat:Dataset');
expect(chk1.identifier).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.license).toBe(null);
expect(chk1.license).toBe('');
expect(chk1.landingPage).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.title).toBe('Tahoe places of interest');
expect(chk1.description).toBe('Description. Here be Tahoe things. You can do a lot here. Here are some more words. And a few more.<div><br /></div><div>with more words</div><div><br /></div><div>adding a few more to test how long it takes for our jobs to execute.</div><div><br /></div><div>Tom was here!</div>');
Expand Down Expand Up @@ -74,7 +74,7 @@ describe('generating DCAT-US 1.1 feed', () => {

expect(chk1['@type']).toBe('dcat:Dataset');
expect(chk1.identifier).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.license).toBe(null);
expect(chk1.license).toBe('');
expect(chk1.landingPage).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.title).toBe('Tahoe places of interest');
expect(chk1.description).toBe('Tahoe places of interest');
Expand Down Expand Up @@ -113,7 +113,7 @@ describe('generating DCAT-US 1.1 feed', () => {

expect(chk1['@type']).toBe('dcat:Dataset');
expect(chk1.identifier).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.license).toBe(null);
expect(chk1.license).toBe('');
expect(chk1.webService).toBe(undefined);
expect(chk1.landingPage).toBe('https://download-test-qa-pre-a-hub.hubqa.arcgis.com/datasets/f4bcc1035b7d46cba95e977f4affb6be_0');
expect(chk1.title).toBe('Tahoe places of interest');
Expand All @@ -139,17 +139,21 @@ describe('generating DCAT-US 1.1 feed', () => {

const expectedDependencies = [
'id',
'licenseInfo',
'structuredLicense',
'layer.geometryType',
'server.spatialReference',
'metadata.metadata.distInfo.distTranOps.onLineSrc',
'url',
'name',
'description',
'tags',
'created',
'modified',
'orgContactEmail',
'source',
'owner'
'owner',
'extent'
];

expect(dependencies.length).toBe(expectedDependencies.length);
Expand All @@ -174,15 +178,19 @@ describe('generating DCAT-US 1.1 feed', () => {

const expectedDependencies = [
'id',
'licenseInfo',
'structuredLicense',
'layer.geometryType',
'server.spatialReference',
'metadata.metadata.distInfo.distTranOps.onLineSrc',
'url',
'name',
'description',
'created',
'modified',
'owner',
'orgContactEmail',
'extent',
'keyword',
'other.source',
'foo.bar.baz',
Expand Down
2 changes: 2 additions & 0 deletions src/dcat-us/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export function getDataStreamDcatUs11(siteItem: IItem, dcatCustomizations?: Dcat
stream: new FeedFormatterStream(header, footer, ',\n', formatFn),
dependencies: [
'id', // used for the dataset landing page URL
'licenseInfo', // required for license resolution
'structuredLicense', // required for license resolution
...DISTRIBUTION_DEPENDENCIES,
...listDependencies(datasetTemplate)
]
Expand Down
3 changes: 1 addition & 2 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ describe('Output Plugin', () => {
},
options: {
portal: 'https://www.arcgis.com',
fields: 'id,layer,server,metadata,name,description,tags,created,modified,source,owner,orgContactEmail',
},
fields: "id,licenseInfo,structuredLicense,layer,server,metadata,url,name,description,tags,created,modified,source,owner,orgContactEmail,extent" },
});
});

Expand Down