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

Javascript wrapper for collectives API #988

Merged
merged 1 commit into from
Dec 25, 2023
Merged

Conversation

max-nextcloud
Copy link
Collaborator

@max-nextcloud max-nextcloud commented Nov 6, 2023

Wrap all usage of the collectives api in a simple functional interface.
This will allow us to change the layout of the api
and only update it in a single place.

Refactor client (browser) app

Separate the API handling from the store.
The api is concerned with axios and generateUrl
while the store is only concerned with the current state of the app.

Url generation from parts

Accept multiple arguments to collectivesUrl and pagesUrl
and join them with / to build the url:

Example:

api.pagesUrl({collectiveId: 10}, 'trash', 12)

results in /apps/collectives/_api/10/_pages/trash/12

Tests (Cypress)

Use the api directly in the cypress commands.
That way we do not require the page to be loaded.

Also extract collective-edit-mode.spec.js from pages.spec.js.
It requires a different beforeEach function
to seed the edit mode before visiting the collective.

Signed-off-by: Max [email protected]

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

This comment was marked as outdated.

@max-nextcloud max-nextcloud force-pushed the test/cypress-to-api branch 4 times, most recently from beae1fa to 2eab339 Compare November 15, 2023 09:14
@max-nextcloud max-nextcloud force-pushed the test/cypress-to-api branch 7 times, most recently from a3ef90a to ea9fb8a Compare November 27, 2023 22:00
@max-nextcloud max-nextcloud marked this pull request as ready for review November 28, 2023 07:04
@max-nextcloud max-nextcloud requested a review from mejo- November 28, 2023 07:07
@max-nextcloud
Copy link
Collaborator Author

@mejo- I still have some more ideas here but i feel like this could already go in and I can open a new PR for the next steps.

I'd be happy about a review if you can spare some time. If you're busy with other things that's also fine. 🙂

@max-nextcloud max-nextcloud force-pushed the test/cypress-to-api branch 3 times, most recently from e697fe1 to 6f719ff Compare November 29, 2023 05:01
@max-nextcloud max-nextcloud changed the title test(cy): extract api from vuex to use in cy commands Javascript wrapper for collectives API Nov 29, 2023
@max-nextcloud max-nextcloud force-pushed the test/cypress-to-api branch 8 times, most recently from 4028334 to 282a272 Compare December 1, 2023 11:07
@mejo- mejo- force-pushed the test/cypress-to-api branch 2 times, most recently from 075c0de to 9fe4d6e Compare December 5, 2023 18:47
@mejo-
Copy link
Member

mejo- commented Dec 5, 2023

@max-nextcloud after rebase quite a few cypress tests fail now on all NC versions. Could you take a look?

@max-nextcloud max-nextcloud force-pushed the test/cypress-to-api branch 2 times, most recently from 227e700 to 1bdda92 Compare December 23, 2023 16:05
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

I skimmed through the changes and they look good to me. Thanks a lot for all the refactoring work, the whole requests logic in frontend is so much cleaner now ❤️

Wrap all usage of the collectives api in a simple functional interface.
This will allow us to change the layout of the api
and only update it in a single place.

Separate the API handling from the store.
The api is concerned with axios and generateUrl
while the store is only concerned with the current state of the app.

Accept multiple arguments to `collectivesUrl` and `pagesUrl`
and join them with `/` to build the url:

Example:
```js
api.pagesUrl({collectiveId: 10}, 'trash', 12)
```
results in `/apps/collectives/_api/10/_pages/trash/12`

Use the api directly in the cypress commands.
That way we do not require the page to be loaded.

Also extract `collective-edit-mode.spec.js` from `pages.spec.js`.
It requires a different beforeEach function
to seed the edit mode before visiting the collective.

Signed-off-by: Max <[email protected]>
@max-nextcloud max-nextcloud merged commit b6336b5 into main Dec 25, 2023
52 checks passed
@delete-merged-branch delete-merged-branch bot deleted the test/cypress-to-api branch December 25, 2023 12:29
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.

2 participants