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

Jump to bottom

Update gn4 API client to 4.2.7 #718

Merged
merged 16 commits into from
Dec 11, 2023
Merged

Update gn4 API client to 4.2.7 #718

merged 16 commits into from
Dec 11, 2023

Conversation

fgravin
Copy link
Member

@fgravin fgravin commented Dec 9, 2023

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 9, 2023 •
edited
Loading

Affected libs: api-repository, feature-catalog, feature-record, feature-router, feature-search, feature-map, feature-dataviz, feature-auth, common-domain, api-metadata-converter, feature-editor, ui-search, common-fixtures, ui-elements, ui-catalog, util-shared, ui-widgets, ui-inputs, ui-map, ui-dataviz, data-access-gn4,
Affected apps: datahub, metadata-editor, demo, webcomponents, search, map-viewer, datafeeder, metadata-converter, data-platform,

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

@coveralls
Copy link

coveralls commented Dec 9, 2023 •
edited
Loading

Coverage Status

coverage: 82.953% (-0.9%) from 83.889%
when pulling 10f0755 on gn4-api-update
into 10efb71 on main.

@fgravin fgravin mentioned this pull request Dec 9, 2023
Merged
5 tasks
jahow
jahow reviewed Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

Thanks! it'd be better if there was no manual change required in there, is it because the openapi generator is not working correctly? Did you try updating it maybe? (theee's a new minor version available)

Also the e2e tests failure should be fixed before merging, it doesn't look like the usual flaky one.

libs/data-access/gn4/src/openapi/api/records.api.service.ts
import {
AssociatedRecordApiModel,
BatchEditParameterApiModel,
} from '../model/models'
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is that change necessary? Having manual changes in there is going to bite us later if we keep updating the client automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had 2 issues after the generation

  • AssociatedRecordApiModel was not imported in this file
  • a problem in the model here generated by types.

So I fixed them manually.
It could also be the open api spec.

@fgravin
Copy link
Member Author

fgravin commented Dec 11, 2023

Also the e2e tests failure should be fixed before merging, it doesn't look like the usual flaky one.

Fixed in the related PR 7d3e1e1

fgravin added 15 commits December 11, 2023 13:09
@fgravin
chore: update gn4 API client to 4.2.7
2bfc1af
@fgravin
fix(gn4-api): search now takes 'relatedTypes' as second argument
c08898e
@fgravin
fix(gn4-api): add missing import to AssociatedRecordApiModel
aeadcd9
@fgravin
fix(gn4-api): cast getSourceByType to CatalogSource[]
1f11f0f 
There is an issue in the openapi spec about the response model
@fgravin
test(gn4-api): fix API changes within the tests
a7b1314
@fgravin
refactor(fields): rename GnU1Translated to KeySearchField
becd257 
as we search on a key term, not on a simple term
@fgravin
refactor(fields): remove gn4 ToolsApi dependency
72cfefc 
fields is in the domain and should not depend on gn4 repository implementation.
this should pass through the platform interface abstraction
@fgravin
feat(platform): add getThesaurusByLang method
476ab57 
to fetch all keywords in the requested language.
Implementation done for gn4, note that it uses a search request, with a limit of 1000, will be an issue in the long run
@fgravin
feat(fields): add a thesaurus field for inspire
48127de
@fgravin
fix(quality): handle i18n in quality widget items
a7ba969
@fgravin
fix(figures): handle i18n for titles
cf13f70
@fgravin
fix(i18n): regenerate translations, from fixes
b2e49f5
@fgravin
feat(filters): translate contact filter
9e104c9
@fgravin
feat(thesaurus): add model
8010696
@fgravin
fix(sources): use back /sources request
90a9471 
as /sourceS/harvester is not compatible in 4.2.2
@fgravin
Copy link
Member Author

fgravin commented Dec 11, 2023

Fixed in the related PR 7d3e1e1

Dependency PR merged and code rebased.
Ready for merge into main after that the CI passes.

jahow
jahow approved these changes Dec 11, 2023
View reviewed changes
Copy link
Collaborator

@jahow jahow left a comment

Choose a reason for hiding this comment

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

thanks!

@fgravin fgravin merged commit 92348fa into main Dec 11, 2023
8 checks passed
@fgravin fgravin deleted the gn4-api-update branch December 11, 2023 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jahow jahow jahow approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fgravin @coveralls @jahow