Skip to content

Fix warning wrongly appearing on template change#1584

Open
Arnei wants to merge 1 commit intoopencast:r/19.xfrom
Arnei:warning-wrong-on-template-change
Open

Fix warning wrongly appearing on template change#1584
Arnei wants to merge 1 commit intoopencast:r/19.xfrom
Arnei:warning-wrong-on-template-change

Conversation

@Arnei
Copy link
Copy Markdown
Member

@Arnei Arnei commented Apr 23, 2026

When changing the template in the acl tab, if the current acl settings were invalid, a warning would appear after the template change about how the acl was invalid, when due to the template change it actually no longer wasn't.
This fixes that.

How to test this

Get your ACL to be invalid (by removing all roles, or by adding a new role and not selecting a role in the dropdown). Then change template.

When changing the template in the acl tab, if the current acl
settings were invalid, a warning would appear *after* the template
change about how the acl was invalid, when due to the template
change it actually no longer wasn't.
This fixes that.
@Arnei Arnei added the type:bug Something isn't working label Apr 23, 2026
@github-actions
Copy link
Copy Markdown
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/admin-interface:pr-1584

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/admin-interface:pr-1584

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.

@github-actions
Copy link
Copy Markdown
Contributor

This pull request is deployed at test.admin-interface.opencast.org/1584/2026-04-23_08-42-31/ .
It might take a few minutes for it to become available.

@gregorydlogan gregorydlogan self-assigned this May 4, 2026
@gregorydlogan
Copy link
Copy Markdown
Member

Maybe I'm doing something wrong here, but the default private ACL has no roles, so I immediately get the modal's error message. I can't imagine private actually works correctly the more I think about it, but I'm not sure if we should remove it, or fix it somehow.

@Arnei
Copy link
Copy Markdown
Member Author

Arnei commented May 6, 2026

At the very least we should not claim that an ACL needs at least one read/write role, and then violate that claim by having a template that is literally an empty ACL. Maybe add ROLE_ADMIN with read/write to private? Should effectively be the same, right?

@gregorydlogan
Copy link
Copy Markdown
Member

That would be functionally identical, but likely confusing for the user since they aren't listed in their own ACL...

Could we have the UI default to showing the user as an at-least-read role configuration if an empty template is selected? If the user manually removes all of the roles the behaviour is correct, it's just loading an empty template that causes weirdness - I would expect that both

a) all of the templates work, even if they don't do what I want (ie, I can lock myself out of my own video)
b) I can't deliberately set ACLs such that no one aside from the global admin can help me.

I guess there's a discussion to be had about point a. Should we even allow that?

@Arnei
Copy link
Copy Markdown
Member Author

Arnei commented May 7, 2026

The user itself is not listed in any of the templates. But the ACL tab in the event/series details makes sure to add the role of the user on template change, so that they are listed in their own ACL. If that does not happen for private, that is a bug actually. Which is not something this PR has to fix actually :P

I am not an admin, but from what I hear, you basically never want to lock yourself out of your own video. It probably should remain technically possible for some edge cases I can't even imagine. But from a usability standpoint we should ensure that a user never accidentally loses rights on their own video, only very deliberately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants