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 metadata editting #3342

Merged
merged 1 commit into from
Jan 26, 2022
Merged

Conversation

lkiesow
Copy link
Member

@lkiesow lkiesow commented Jan 10, 2022

This patch removes a check which would prevent users from editing
metadata of scheduled events which start time has already passed.

Editing such metadata in the admin interface will fail silently with no
feedback for users but many errors in the logs:

2022-01-10 21:42:32,986 | INFO  | (SchedulerServiceImpl:837) - Event ID ba78ec72-6d8c-40f8-807c-67d6a6764a39 has already ended as its end time was 2022-01-10T16:10:00Z and current time is 2022-01-10T20:42:32Z
2022-01-10 21:42:32,987 | ERROR | (IndexServiceImpl:1303) - Unable to update scheduled event ba78ec72-6d8c-40f8-807c-67d6a6764a39 with metadata [...] because
org.opencastproject.scheduler.api.SchedulerException: Event ID ba78ec72-6d8c-40f8-807c-67d6a6764a39 has already ended at 2022-01-10T16:10:00Z and now is 2022-01-10T20:42:32Z
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.verifyActive(SchedulerServiceImpl.java:840) ~[?:?]
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.updateEventInternal(SchedulerServiceImpl.java:671) ~[?:?]
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.updateEvent(SchedulerServiceImpl.java:619) ~[?:?]

I see no reason why we shouldn't allow users to edit e.g. the title of
such events and this patch now makes it possible.

Your pull request should…

This patch removes a check which would prevent users from editing
metadata of scheduled events which start time has already passed.

Editing such metadata in the admin interface will fail silently with no
feedback for users but many errors in the logs:

```
2022-01-10 21:42:32,986 | INFO  | (SchedulerServiceImpl:837) - Event ID ba78ec72-6d8c-40f8-807c-67d6a6764a39 has already ended as its end time was 2022-01-10T16:10:00Z and current time is 2022-01-10T20:42:32Z
2022-01-10 21:42:32,987 | ERROR | (IndexServiceImpl:1303) - Unable to update scheduled event ba78ec72-6d8c-40f8-807c-67d6a6764a39 with metadata [...] because
org.opencastproject.scheduler.api.SchedulerException: Event ID ba78ec72-6d8c-40f8-807c-67d6a6764a39 has already ended at 2022-01-10T16:10:00Z and now is 2022-01-10T20:42:32Z
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.verifyActive(SchedulerServiceImpl.java:840) ~[?:?]
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.updateEventInternal(SchedulerServiceImpl.java:671) ~[?:?]
        at org.opencastproject.scheduler.impl.SchedulerServiceImpl.updateEvent(SchedulerServiceImpl.java:619) ~[?:?]
```

I see no reason why we shouldn't allow users to edit e.g. the title of
such events and this patch now makes it possible.
@lkiesow lkiesow added the bug label Jan 10, 2022
@ziegenberg
Copy link
Member

How about making this configurable?

@lkiesow
Copy link
Member Author

lkiesow commented Jan 12, 2022

I don't think that's a good idea. Assuming it's allowed by default, users would probably complain if they disallow that and then the user interface starts misbehaving, and your log gets spammed by errors. I don't want to build something like that. And that is apart from this working different in different Opencast's being quite confusing.

Does your question mean you don't want this to be merged?

@gregorydlogan
Copy link
Member

I know we discussed this in the technical meeting, but I don't remember the resolution. This can/will cause issues with CAs which allow for editing metadata in the classroom (c.f Galicaster). Were we just going to ignore that any let the user deal with conflicting metadata?

@lkiesow
Copy link
Member Author

lkiesow commented Jan 20, 2022

Such a CA can always overwrite Opencast's metadata. What we have now is just a partial protection against conflicts by basically disallowing anyone on the server side to edit metadata via the admin interface by breaking the interface during that period (backend will return an error; the interface is not doing anything and giving no proper feedback). From a user's perspective, I think this just looks broken. As I mentioned, it's also only a partial protection since we don't know when the agent last requested its data, so if you actually want to protect against conflicts which can occur due to capture agents overwriting the data, you would need to lock this once a capture agent has requested the data for the first time.

Long story short, I'm in favor for applying this patch.

That said, I don't feel that strongly about this and if you all rather live with no editing capabilities and a partially broken user interface, I'm fine with that. Just decline this. I won't get angry about that. In the end, it's just an edge case anyway, and people will not that often edit metadata past the event's start date.

One thing I do feel strongly about is making this configurable. That, I think, is just confusing for users and capture agent vendors and feels unnecessary.

@gregorydlogan
Copy link
Member

This sounds like a reasonable change then. Strongly agree with not making this configurable. I suspect the intersection between adopters who edit metadata that close to a recording, and adopters who both have Galicaster and expose its UI to their users and those users make changes is... small. If this becomes an issue further on then we can have a larger discussion, but in the meantime this resolves a UI oddity.

@gregorydlogan gregorydlogan self-assigned this Jan 26, 2022
@gregorydlogan gregorydlogan merged commit f0494fc into opencast:r/11.x Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants