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

Feature/update apis to v1 #849

Closed
wants to merge 9 commits into from
Closed

Feature/update apis to v1 #849

wants to merge 9 commits into from

Conversation

lilachic
Copy link
Contributor

@lilachic lilachic commented Jun 16, 2022 •
edited
Loading

Description

Add /v1 in API urls to adapt backend change.

Related issues

Fixes #826

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update (this needs a follow up PR)

Changes Made

The frontend uses now following APIs
"/v1/objects"
"/v1/schemas"
"/v1/validate"
"/v1/submit"
"/v1/submissions"
"/v1/publish"
"/v1/drafts"
"/aai"
"/callback"
"/logout"
"/v1/users"
"/v1/templates"

Added a line about /v1 in API urls in CHANGELOG.
Rename cookie MTD_SESSION to AOIHTTP_SESSION in e2e login test.

Testing

  • Unit Tests
  • Integration Tests

Mentions

@lilachic lilachic requested a review from hannyle June 16, 2022 15:50
@lilachic lilachic self-assigned this Jun 16, 2022
@lilachic lilachic linked an issue Jun 16, 2022 that may be closed by this pull request
API moved to /v1 #826
Open
@lilachic lilachic marked this pull request as draft June 16, 2022 15:53
@csc-felipe csc-felipe marked this pull request as ready for review June 22, 2022 11:30
@csc-felipe csc-felipe force-pushed the feature/update-apis-to-v1 branch from 446d3fc to 8f1b815 Compare June 22, 2022 11:50
@csc-felipe
Copy link
Contributor

Rebased and pushed, to see test results.

@csc-felipe csc-felipe force-pushed the feature/update-apis-to-v1 branch from 8f1b815 to 6be2240 Compare June 22, 2022 11:53
@csc-felipe
Copy link
Contributor

Had to update node, with new npm, same version as github actions, I get the same errors:

$ nvm install 16
$ nvm use 16.15.1
$ node --version
v16.15.1
$ npm --version
8.11.0

@csc-felipe csc-felipe force-pushed the feature/update-apis-to-v1 branch from 6be2240 to b2f7e01 Compare June 22, 2022 12:08
@csc-felipe
Copy link
Contributor

Looking at the changes made, I think it still needs something to be done about the routing, and checking that user is logged in. Assuming these were not there before.

@lilachic
Copy link
Contributor Author

Headless run of the Cypress tests locally fail with login "│ ✖ login.spec.ts ". Locally even e2etest run one by one pass. Running 15 integration tests broke Chrome.
Did not help locally to run npm install with the following versions, the commit "update package-lock.json" 446d3fc:
node-v
v16.14.0
npm -v
8.3.1
So more work with logout, and maybe a new PR for routing.

@lilachic lilachic force-pushed the feature/update-apis-to-v1 branch from b4d29ce to a133eee Compare August 19, 2022 08:35
@lilachic
Copy link
Contributor Author

The e2e tests to pass need the CSCfi/metadata-submitter#554 merged in the backed develop branch. Locally with that branch the tests passes.

@csc-felipe
Copy link
Contributor

The e2e tests to pass need the CSCfi/metadata-submitter#554 merged in the backed develop branch. Locally with that branch the tests passes.

Done

@lilachic lilachic force-pushed the feature/update-apis-to-v1 branch from 9734942 to 768c6a7 Compare August 22, 2022 14:04
@lilachic
Copy link
Contributor Author

No tests fail for getting 400 from v1/schemas/datacite .

@csc-felipe
Copy link
Contributor

No tests fail for getting 400 from v1/schemas/datacite .

I'm sorry, but I didn't understand. Do you mean that /v1/schemas/datacite still gives HTTP 400?

@lilachic lilachic force-pushed the feature/update-apis-to-v1 branch from 432e2a7 to b12db04 Compare September 14, 2022 16:18
@hannyle hannyle closed this Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hannyle hannyle Awaiting requested review from hannyle

Assignees

@lilachic lilachic

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API moved to /v1
3 participants
@lilachic @csc-felipe @hannyle