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

ME: edit record distributions #1002

Merged
merged 2 commits into from
Sep 30, 2024
Merged

ME: edit record distributions #1002

merged 2 commits into from
Sep 30, 2024

Conversation

LHBruneton-C2C
Copy link
Collaborator

@LHBruneton-C2C LHBruneton-C2C commented Sep 23, 2024

Description

This PR introduces the section on associated resources, allowing to add both download and service as distributions.

TODO: split and share components between annexes and associated resources.

Screenshots

image
image

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Affected libs: feature-editor, ui-inputs, feature-dataviz, feature-record, feature-router, feature-search, feature-map, ui-elements, feature-notifications, feature-catalog, ui-catalog, ui-search, ui-layout,
Affected apps: metadata-editor, datafeeder, demo, metadata-converter, datahub, webcomponents, search, map-viewer,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch from a45da7a to c29ab22 Compare September 23, 2024 13:58
@@ -381,6 +381,7 @@ export function extractDatasetOnlineResources(
mapArray(
([isService, isDownload, protocol, url, name, description, mimeType]) => {
if (isService) {
//FIXME: allow writing identifier only for wms and wfs?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jahow This is the only point on which I'm not sure how to fix the converter. As it is currently, for service protocols other than WMS and WFS, the identifier is not lost, but it's transferred to the name property.

This is particularly disturbing when reloading a record after it's been first saved as draft. The attributes aren't kept as they were first inputted.

Copy link
Contributor

github-actions bot commented Sep 23, 2024

📷 Screenshots are here!

@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch 2 times, most recently from e91d90d to 2c86d03 Compare September 26, 2024 07:48
@LHBruneton-C2C LHBruneton-C2C marked this pull request as ready for review September 26, 2024 07:48
@LHBruneton-C2C LHBruneton-C2C changed the title wip ME: edit record distributions Sep 26, 2024
@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch 2 times, most recently from 18177ea to 3a6f96c Compare September 26, 2024 12:42
@jahow
Copy link
Collaborator

jahow commented Sep 26, 2024

Did a quick test and found this:

  • When trying to upload a file as a distribution, this error appears in the console and nothing happens:
    image; same for the "attached resources" field below actually

  • When clicking on the main block, the upload file dialog doesn't open

Otherwise looks good! thanks!

Comment on lines +33 to +37
ngOnChanges(changes: SimpleChanges): void {
if (changes.value) {
console.log('changes.value', changes.value)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

to delete

@fvanderbiest
Copy link
Contributor

Just chiming in - "OGC API" title is misleading because there are so many OGC APIs (OGC API Maps, Features, Tiles...)
Have a look at https://ogcapi.ogc.org/#standards

@jahow
Copy link
Collaborator

jahow commented Sep 26, 2024

Just chiming in - "OGC API" title is misleading because there are so many OGC APIs (OGC API Maps, Features, Tiles...) Have a look at https://ogcapi.ogc.org/#standards

On the other hand it makes sense because an OGC API endpoint can provide many different information at the same place I think

@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch from 3a6f96c to f8b9c9d Compare September 26, 2024 14:14
@coveralls
Copy link

coveralls commented Sep 26, 2024

Coverage Status

coverage: 81.027% (-3.9%) from 84.949%
when pulling 177e6ab on ME/record-field-distributions
into be95197 on main.

@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch from f8b9c9d to fc928af Compare September 26, 2024 15:17
@LHBruneton-C2C LHBruneton-C2C force-pushed the ME/record-field-distributions branch from 545d62b to 84ef5d8 Compare September 27, 2024 08:13
@jahow jahow merged commit fdf8f64 into main Sep 30, 2024
13 checks passed
@jahow jahow deleted the ME/record-field-distributions branch September 30, 2024 14:24
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.

4 participants