create api card and form to request Ign download api #989

mmohadIGN · 2024-09-11T14:13:10Z

Description

This PR introduces a new component designed to create an API card for requesting the IGN download API. The component simplifies the integration process by providing a form to interact with the IGN download API.

Architectural changes

Screenshots

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

Details issues #766

jahow · 2024-09-18T08:16:33Z

Hi @mmohadIGN, sorry for the late reply but the team is currently busy working on a first version of the Metadata Editor app, so we simply don't have the bandwidth to look into this PR. Is this OK if the review happens later in October? Thanks

mmohadIGN · 2024-09-18T15:24:59Z

Hi @mmohadIGN, sorry for the late reply but the team is currently busy working on a first version of the Metadata Editor app, so we simply don't have the bandwidth to look into this PR. Is this OK if the review happens later in October? Thanks

Hi @jahow, it's fine, no problem.

CLAassistant · 2024-12-08T03:42:06Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Angi-Kinas

Thanks @mmohadIGN for this contribution and sorry for the delayed review! I haven't tested the feature yet, but the screenshots look good already! :)

There are some minor issues that I think should be addressed.
Also it would be great if we could make the components more generic to fit in this project instead of specifically mentioning IGN. What do you think about this?

Let me know if anything is unclear!

Angi-Kinas · 2024-12-19T15:06:03Z

apps/datahub/src/app/app.module.ts

@@ -162,6 +166,7 @@ export const metaReducers: MetaReducer[] = !environment.production ? [] : []
    UiWidgetsModule,
    RecordMetaComponent,
    LetDirective,
+    MatButtonToggleModule,


Is this really needed here? I don't see it being used.

No, you're right. I removed it

Angi-Kinas · 2024-12-19T15:11:42Z

apps/datahub/src/app/record/record-apis/record-apis.component.html

@@ -36,9 +36,20 @@
  <div class="bg-primary-opacity-10 py-8">
    <div class="flex flex-col px-4 gap-8 container-lg lg:mx-auto">
      <div class="flex flex-wrap justify-between sm:mb-2 ng-star-inserted">
-        <p class="text-[21px] text-title font-title" translate>
+        <p


It looks like the two paragraphs (line 39 -52) are the same except of their text content. Instead of repeating, you could use one paragraph, like so:

Suggested change

<p

<p

class="text-[21px] text-title font-title"

*ngIf="selectedApiLink?.accessServiceProtocol"

translate

>

{{ selectedApiLink?.accessServiceProtocol === 'GPFDL'

? 'record.metadata.api.form.title.gpf'

: 'record.metadata.api.form.title' }}

</p>

If we would have to change the paragraph in the future, we would only have to do that in one place :)

Yes, you're right. I have done it.

Angi-Kinas · 2024-12-19T15:15:58Z

apps/datahub/src/app/record/record-apis/record-apis.component.html

@@ -57,7 +68,13 @@
          </ng-icon>
        </button>
      </div>
+
+      <gn-ui-ign-api-dl


The name of this component gn-ui-ign-api-dl is really specific to IGN. Would there be a way to make the component more generic (maybe reflecting the format it is using?)?
Maybe @jahow has a good idea about it.

Angi-Kinas · 2024-12-19T15:28:48Z

libs/feature/record/src/lib/ign-api-dl/ign-api-dl.component.css

+
+:host ::ng-deep gn-ui-copy-text-button button,
+host ::ng-deep gn-ui-copy-text-button button:hover {
+  background-color: var(--color-secondary) !important;


Is there a way to achieve this without using !important?

Angi-Kinas · 2024-12-19T15:29:05Z

libs/feature/record/src/lib/ign-api-dl/ign-api-dl.component.html

+    </div>
+    <div class="flex flex-row flex-wrap justify-between flex-grow gap-5">
+      <div class="flex flex-col gap-3">
+        <!--<p class="text-sm" translate>record.metadata.api.form.type</p>-->


Is this still needed as it is commented out?

No, you're right. I removed them.

Angi-Kinas · 2024-12-19T15:33:57Z

libs/feature/record/src/lib/ign-api-dl/ign-api-dl.component.ts

+
+  listFilteredProduct$ = this.apiQueryUrl$.pipe(
+    mergeMap((url) => {
+      console.log(url)


console.log ;-)

Angi-Kinas · 2024-12-19T15:35:53Z

libs/feature/record/src/lib/ign-api-dl/ign-api-dl.component.ts

+      }
+      return outputUrl
+    })
+    // startWith(() => this.apiBaseUrl)


Is this still relevant? See also line 126 and 180.

No, I removed it too.

Angi-Kinas · 2024-12-19T15:37:57Z

libs/feature/record/src/lib/ign-api-dl/ign-api-dl.component.ts

+    return choicesTest
+  }
+  async getFields() {
+    this.choices = await this.getCapabilities()


I see a lot of repetition in this part of the code. But maybe I don't understand the logic behind it.
@jahow could we simplify this?

Angi-Kinas · 2024-12-19T15:39:21Z

libs/feature/record/src/lib/ign-api-produit/ign-api-produit.component.html

+    <div *ngFor="let entry of liste$ | async">
+      <a [href]="entry['link'][0]['href']"
+        >{{ entry['link'][0]['href'].split('/').slice(-1) }}
+        <mat-icon


Sorry, we updated our icon library in the meantime and use <ng-icon> now.

Yes, I made the change.

Angi-Kinas · 2024-12-19T15:41:30Z

apps/datahub/src/app/record/record-apis/record-apis.component.ts

@@ -75,6 +76,8 @@ export class RecordApisComponent implements OnInit {
  }

  openRecordApiForm(link: DatasetServiceDistribution) {
+    this.displayApiIgnForm =


This could be simpliefied, as the expression already evaluates to a boolean.

Suggested change

this.displayApiIgnForm =

this.displayApiIgnForm = link.accessServiceProtocol === 'GPFDL';

Yes, you're right. I made the change.

mmohadIGN · 2024-12-19T17:03:56Z

Thanks @mmohadIGN for this contribution and sorry for the delayed review! I haven't tested the feature yet, but the screenshots look good already! :)

There are some minor issues that I think should be addressed. Also it would be great if we could make the components more generic to fit in this project instead of specifically mentioning IGN. What do you think about this?

Let me know if anything is unclear!
Your version is much improved! Here's a slightly polished version to address minor issues:

Hi,

No problem.
Yes, I think generalizing will improve the situation. Do you have any advice on how to proceed?
In the meantime, I’ll correct the issues you pointed out.

Thanks!

Angi-Kinas · 2025-01-06T14:26:02Z

Hello @mmohadIGN, thank you for your patience and thanks for addressing the comments!

I tried to test the new functionality locally and realized that some of the logic interferes with already existing logic of the "Downloads"-section. As a result, some of the downloads now appear in the "API"-section that were in the "Downloads"-section before. For those download resources, there is now the logic available that you created for the IGN downloads, but they don't provide this functionality (see Screenshot):

Therefore I have some questions:

Do you know what the protocol looks like for the IGN API? Is it in any way different from the protocol for other downloads? (e.g. different from WWW:DOWNLOAD-1.0-http--download)
Can you tell me how I can locally test your changes with a dataset from your side? So I can see how it is supposed to work.
Which values will be used for ZONE/ FORMAT / CRS? Do they come from the API?

In the meantime, maybe you can adjust the placing of the button ("reset") and text ("create your request") above the dropdowns a bit?
Currently it looks a bit squished together:

But it could look like the other API form (record-api-form.component):

Additionally my team had an idea about making the naming more generic: Could you please change the name of the components that contain ign into gpf or gpfdl? (e.g. gpf-api-dl.component.ts)

Let me know if you have any questions.
Thanks!

mmohadIGN · 2025-01-08T15:02:51Z

Hello @Angi-Kinas,

I made the style change you request and rename the file.
About the http-download protocols, maybe I can check with a regex the URL ?
For example, a metadata : https://data.geopf.fr/csw?REQUEST=GetRecordById&SERVICE=CSW&VERSION=2.0.2&OUTPUTSCHEMA=http://standards.iso.org/iso/19115/-3/mdb/2.0&elementSetName=full&ID=IGNF_BD-TOPO
Can you use this metadata for your test ?

Add(IGN-dwonload): add api card to request Ign download api

ecda702

mmohadIGN mentioned this pull request Sep 11, 2024

[Datahub] - Add filter options on the download menu #766

Open

Merge branch 'main' into IGN_DL

666bba1

jahow added this to the 2.4.0 milestone Nov 14, 2024

Merge branch 'main' into IGN_DL

498c1db

Angi-Kinas reviewed Dec 19, 2024

View reviewed changes

mmohadIGN added 10 commits December 27, 2024 14:57

fix(import): fix wrong import

1be8aef

fix(app): supress import not use

1f0751b

fix(ign-dl): supress hello

4051f5a

fix(ign-dl):supress console.log

d11d3ae

fix(records): pass two paragraph to one

3c5b834

fix(comment): supress not use comment

44d702e

fix(ign): change to upper case

5e63906

fix(comment): supress not use comment

2d4352e

fix(icon): migrate mat to ng-icon

b6c4635

change(css): supress !important

531f2a0

mmohadIGN added 2 commits January 8, 2025 14:45

rename(gpf-api): renmae file ign-api-dl to gpf-api-dl

0809a21

style(gpf-dla): make the form look less squished

9042df6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create api card and form to request Ign download api #989

create api card and form to request Ign download api #989

mmohadIGN commented Sep 11, 2024 •

edited

Loading

jahow commented Sep 18, 2024

mmohadIGN commented Sep 18, 2024

CLAassistant commented Dec 8, 2024

Angi-Kinas left a comment

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

Angi-Kinas Dec 19, 2024

Angi-Kinas Dec 19, 2024 •

edited

Loading

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

Angi-Kinas Dec 19, 2024

mmohadIGN Dec 27, 2024

mmohadIGN commented Dec 19, 2024

Angi-Kinas commented Jan 6, 2025 •

edited

Loading

mmohadIGN commented Jan 8, 2025

-        <p
+        <p
+  class="text-[21px] text-title font-title"
+  *ngIf="selectedApiLink?.accessServiceProtocol"
+  translate
+>
+  {{ selectedApiLink?.accessServiceProtocol === 'GPFDL'
+      ? 'record.metadata.api.form.title.gpf'
+      : 'record.metadata.api.form.title' }}
+</p>

	this.displayApiIgnForm =
	this.displayApiIgnForm = link.accessServiceProtocol === 'GPFDL';

create api card and form to request Ign download api #989

Are you sure you want to change the base?

create api card and form to request Ign download api #989

Conversation

mmohadIGN commented Sep 11, 2024 • edited Loading

Description

Architectural changes

Screenshots

Quality Assurance Checklist

jahow commented Sep 18, 2024

mmohadIGN commented Sep 18, 2024

CLAassistant commented Dec 8, 2024

Angi-Kinas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Angi-Kinas Dec 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmohadIGN commented Dec 19, 2024

Angi-Kinas commented Jan 6, 2025 • edited Loading

mmohadIGN commented Jan 8, 2025

mmohadIGN commented Sep 11, 2024 •

edited

Loading

Angi-Kinas Dec 19, 2024 •

edited

Loading

Angi-Kinas commented Jan 6, 2025 •

edited

Loading