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

Allow series path removal via Tobira tab in series details #983

Merged
merged 6 commits into from
Dec 11, 2024

Conversation

owi92
Copy link
Contributor

@owi92 owi92 commented Nov 19, 2024

This modifies the UI from #878 to include the option to remove a series path in Tobira ("unmounting").
Needs opencast/opencast#6317 to work.

Also fixes a minor bug and introduces icon buttons for the series path editing (see screenshots):
Bildschirmfoto 2024-11-19 um 16 19 53
Bildschirmfoto 2024-11-19 um 16 19 58
Bildschirmfoto 2024-11-19 um 16 20 15
Bildschirmfoto 2024-11-19 um 16 20 20

I introduced said error with a previous fix for another bug...
Also adds some padding for lenghtier texts in the modal.
This should help to more accurately distinguish the
editing of existing paths and the mounting previously
unmounted series (either when creating a series or working
on a series that just wasn't mounted when originally created).
These cases need slightly different texts.
Also simplifies some markup of the Tobira path table and
introduces the use of icon buttons for editing and removing
paths (instead of link-styled "text" buttons).
@Arnei Arnei added the type:enhancement New feature or request label Nov 19, 2024
Copy link
Contributor

This pull request is deployed at test.admin-interface.opencast.org/983/2024-11-19_15-21-28/ .
It might take a few minutes for it to become available.

Copy link
Contributor

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-983

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-983

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Works and looks reasonable to me.

I changed the mount endpoint in opencast,
so now this uses a dedicated endpoint each for
mounting and unmounting.
Copy link
Contributor

github-actions bot commented Dec 10, 2024

This pull request is deployed at test.admin-interface.opencast.org/983/2024-12-10_21-34-56/ .
It might take a few minutes for it to become available.

Copy link
Contributor

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-983

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-983

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

@owi92
Copy link
Contributor Author

owi92 commented Dec 10, 2024

I split up the series endpoint in opencast/opencast#6317. There is now a dedicated endpoint for removing the path, and my latest push here utilizes that endpoint.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Bildschirmfoto vom 2024-12-10 15-41-34
If I open a series details tab like the above, close it, then open the series details tab of another series, the series details tab of that other series will also look like above, even if it normally wouldn't. This can only be fixed with a page reload.

Furthermore, it seems I cannot update a series path anymore. The web request to do so fails with a 400.
Request:
http://localhost:3000/admin-ng/series/84e3ddbf-c799-41ac-83b5-22e2e4b23e8f/tobira/path

{
	"pathComponents": "[{\"name\":\"uiuiui\",\"pathSegment\":\"uiuiui\"}]",
	"targetPath": "/uiuiui"
}

@owi92
Copy link
Contributor Author

owi92 commented Dec 10, 2024

If I open a series details tab like the above, close it, then open the series details tab of another series, the series details tab of that other series will also look like above, even if it normally wouldn't. This can only be fixed with a page reload.

huh dang, once again I can't reproduce that, but I slightly remember running into that in the past.

The other thing should be fixed with my latest push of opencast/opencast#6317. Please don't look at the diff. It's really stupid.

@owi92
Copy link
Contributor Author

owi92 commented Dec 10, 2024

Ok, I did just manage to reproduce the first issue.

  • First I need to create a series without choosing a path for it in tobira. Apparently, Tobira doesn't like that and fails to create the series in its DB (it used to be able to handle this case, need to investigate)
  • Then opening the series details of the newly created series will result in your screenshot above
  • The Tobira tab of other events will also be broken, like you noted above

The Tobira thing should be an easy enough fix, but that still leaves the other issue.
Let me look into it some more.

@owi92 owi92 marked this pull request as draft December 10, 2024 16:41
This will basically just reset any Tobira errors
when opening a new series details modal, and
remove any notifications when closing series modals.
@owi92 owi92 marked this pull request as ready for review December 10, 2024 21:36
@owi92
Copy link
Contributor Author

owi92 commented Dec 10, 2024

Alright, the error should be fixed now.

Some background information:
When a new series is created without setting a path for Tobira, the series won't be immediatly created in Tobira, but it will eventually get in there when sync happens.
In a dev environment, sync will only happen if you manullay run cargo run -- worker in Tobira's backend. In production, this should be always be running, but it might take several minutes, depending on the configured poll period.
So as long as Tobira doesn't know about the series, an error is set and the notification from your screenshot is shown. (This has "always" been like that and I don't think it needs to be changed, at least not in this PR)

The issue you noticed when opening another details modal of a series already known to Tobira was happening because the error and notification were not reset when closing a modal.
I don't have a lot of experience with redux, so I am not sure if the removeNotification dispatch and error reset are done in the correct places now (there are multiple options), but so far I didn't notice any unwanted side effects with my fix.

Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Issues have been fixed, lgtm.

@Arnei Arnei merged commit 1df75da into opencast:main Dec 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants