-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix duplicated links OGC API #1134
Conversation
Affected libs:
|
📷 Screenshots are here! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I added a few comments, I think we're using the wrong field from the collection info. Let me know what you think
@@ -187,6 +188,7 @@ export class DataService { | |||
return Object.keys(collectionInfo.bulkDownloadLinks).map((downloadLink) => { | |||
return { | |||
...ogcApiLink, | |||
name: collectionInfo.title, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't you need to use id
here? I think title
is the human readable label, so it won't match with the WFS feature type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to id, both works
getDownloadUrlsFromOgcApi = jest.fn((url: string) => | ||
url.indexOf('error') > -1 | ||
? Promise.reject(new Error('ogc.unreachable.unknown')) | ||
: Promise.resolve({ | ||
name: 'collection1', | ||
title: 'Collection 1', | ||
bulkDownloadLinks: { | ||
'application/geo+json': 'http://example.com/collection1.geojson', | ||
'application/json': 'http://example.com/collection1.json', | ||
}, | ||
}) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you need to add this change to the test? Is this an improvement to the existing test or was thee a failure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i make the mock in the wrong test and i forgot to delete I think. The goal is to see the forcing of the collection name... in the data service spec not this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! it works :)
Description
This PR is about a bug encontred at MEL. The OGC and WFS links where duplicated because the OGC api does not have a name property linkd to the dataset. So to avoid this, in both OGCApi and WFS services, i reset the collection name. Now both OGC and WFS collection will have the same name, and the duplication remover works.
Architectural changes
The following library now depends on [...]
Screenshots
[...]
Quality Assurance Checklist
breaking change
labelbackport <release branch>
label