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

Change mount_series endpoint to accept and move existing series #1225

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

owi92
Copy link
Member

@owi92 owi92 commented Aug 12, 2024

This is necessary for an upcoming admin UI feature which will allow admins to change the path of series pages with no other blocks (part of opencast/opencast-admin-interface#311).

This shouldn't break any existing behaviour.
Related Opencast and admin UI PRs: opencast/opencast#6091, opencast/opencast-admin-interface#878.

@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 12, 2024 09:19 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from a123e17 to 6a2d43e Compare August 12, 2024 16:57
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 12, 2024 16:59 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from fe39123 to a9556b3 Compare August 13, 2024 09:20
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 09:26 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from a9556b3 to 156f1ab Compare August 13, 2024 09:27
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 09:30 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from 156f1ab to dfc0d4d Compare August 13, 2024 11:28
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 11:30 Destroyed
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
@owi92 owi92 force-pushed the mount-and-move-series branch 2 times, most recently from 6a2d43e to fb675ac Compare August 13, 2024 12:45
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 12:48 Destroyed
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 12:52 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from 0f37033 to fb675ac Compare August 13, 2024 12:53
@owi92 owi92 force-pushed the mount-and-move-series branch from fb675ac to 3a5d83a Compare August 13, 2024 12:57

This comment was marked as resolved.

@github-actions github-actions bot added the status:conflicts This PR has conflicts that need to be resolved label Aug 13, 2024
@owi92 owi92 force-pushed the mount-and-move-series branch from 3a5d83a to a06ba4e Compare August 13, 2024 13:12
@github-actions github-actions bot removed the status:conflicts This PR has conflicts that need to be resolved label Aug 13, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 13:15 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from a06ba4e to 5be04f0 Compare August 13, 2024 13:26

This comment has been minimized.

@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 13:30 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from 5be04f0 to adc795b Compare August 13, 2024 15:02
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 August 13, 2024 15:04 Destroyed
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

We just discussed this in chat, I'm just writing it down here for protocol.

I was still not too happy with very overloaded move API. So the idea now to wait for #1179 and then add three more API endpoints to end up with these:

  • createRealmLineage(realms: { name: string, pathSegment: string })
  • prefigureSeries(series: NewSeries) (not necessarily final name)
  • addSeriesMountPoint(series_oc_id: string, realm_path: string)
  • removeSeriesMountPoint(realm_path: string) (maybe also pass series id to check stuff?)

The admin UI is then changed to send a GraphQL request with multiple mutations in them. GraphQL guarantees us, that these are executed in order. And Tobira uses one DB transaction per GraphQL request, so the whole operation is atomic. Nice!

We do need to keep the old mountSeries endpoint around for some while, until the last admin UI using it is not supported anymore, but that's not a big problem.

Note though that the individual endpoints (especially the MountPoint ones) are still... no atomic perfectly defined operation. It still represents a "high level task" (like the current mountSeries) instead of manipulating Tobira's DB objects directly.

backend/src/api/mutation.rs Outdated Show resolved Hide resolved

This comment was marked as resolved.

@owi92 owi92 force-pushed the mount-and-move-series branch from adc795b to 3f63c30 Compare September 13, 2024 13:06
@github-actions github-actions bot removed the status:conflicts This PR has conflicts that need to be resolved label Sep 13, 2024
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 September 13, 2024 13:12 Destroyed
@owi92 owi92 marked this pull request as draft September 16, 2024 08:33
@owi92 owi92 force-pushed the mount-and-move-series branch from 3f63c30 to 4f86349 Compare September 30, 2024 10:33
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 September 30, 2024 10:41 Destroyed
@owi92 owi92 marked this pull request as ready for review September 30, 2024 10:52
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

I sadly found a few things that need fixing. But I think they should all be straight forward? The general direction is good.

backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/model/realm/mutations.rs Outdated Show resolved Hide resolved
@owi92 owi92 changed the base branch from main to next October 2, 2024 14:30
@owi92 owi92 force-pushed the mount-and-move-series branch from 4f86349 to 6529c59 Compare October 7, 2024 21:31

This comment has been minimized.

@owi92 owi92 force-pushed the mount-and-move-series branch from 6529c59 to bee6021 Compare October 7, 2024 21:31

This comment has been minimized.

@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 October 7, 2024 21:35 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from bee6021 to 482c721 Compare October 7, 2024 21:43
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 October 7, 2024 21:45 Destroyed
@owi92 owi92 force-pushed the mount-and-move-series branch from 482c721 to bd9b1b3 Compare October 7, 2024 21:48
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 October 7, 2024 21:50 Destroyed
Copy link
Member

@LukasKalbertodt LukasKalbertodt left a comment

Choose a reason for hiding this comment

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

Good changes, I only have a few small remarks that should be easy to fix!

backend/src/api/model/series.rs Outdated Show resolved Hide resolved
backend/src/api/model/series.rs Outdated Show resolved Hide resolved
backend/src/api/model/series.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
backend/src/api/mutation.rs Outdated Show resolved Hide resolved
@LukasKalbertodt
Copy link
Member

Oh and: once the admin UI PRs are merged, we should adjust our github action script (check-admin-ui-api.yml) to check the new APIs as well. In another PR tho.

@owi92 owi92 force-pushed the mount-and-move-series branch from bd9b1b3 to 274a1f0 Compare October 8, 2024 10:45
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 October 8, 2024 10:51 Destroyed
This is necessary for an upcoming admin UI feature which will allow
admins to change the path of series pages with no other blocks.
@owi92 owi92 force-pushed the mount-and-move-series branch from 274a1f0 to 1bf3720 Compare October 8, 2024 11:31
@github-actions github-actions bot temporarily deployed to test-deployment-pr1225 October 8, 2024 11:34 Destroyed
@LukasKalbertodt LukasKalbertodt merged commit fe23b4e into elan-ev:next Oct 8, 2024
4 checks passed
KatrinIhler added a commit to opencast/opencast that referenced this pull request Oct 29, 2024
This adds
 - a) an endpoint to get paths of pages in Tobira that host an event
 - b) an endpoint to update the path of a series in Tobira

This shouldn't break any existing behaviour.

These are needed for additional requirements of
opencast/opencast-admin-interface#311.
Corresponding Tobira and admin-UI PRs:
elan-ev/tobira#1225,
opencast/opencast-admin-interface#878.

### Your pull request should…

* [ ] have a concise title
* [ ] [close an accompanying
issue](https://docs.opencast.org/develop/developer/#participate/development-process/#automatically-closing-issues-when-a-pr-is-merged)
if one exists
* [ ] [be against the correct
branch](https://docs.opencast.org/develop/developer/development-process#acceptance-criteria-for-patches-in-different-versions)
* [ ] include migration scripts and documentation, if appropriate
* [ ] pass automated tests
* [ ] have a clean commit history
* [ ] [have proper commit messages (title and body) for all
commits](https://medium.com/@steveamaza/e028865e5791)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:dev Changes primarily for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants