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

(3/3) Annotation template feature #605

Conversation

unterrichtsvideos
Copy link

@unterrichtsvideos unterrichtsvideos commented Apr 3, 2023

This branch builds upon the merged+ bugfixed branch from PR #603 and PR #604

⚠️ This is the tested final version. However, if the bugfixes branch changes during QA and before it is merged into the final master, then this needs to be updated to.

mlunzena and others added 30 commits May 26, 2020 08:00
`label.category` is only set for preexisting categories,
not for ones created during the current browser session.
With this patch, the frontend also knows about deleted labels,
categories, scales and scale values. This makes the behavior of
annotations carrying any of that deleted structure much more
consistent.
Previously it already showed the editing interface for the scale
of the category you got to the dialog with. It wasn't fully in edit
mode yet, though, so edits could get lost.
@JulianKniephoff JulianKniephoff deleted the branch opencast:master July 17, 2023 08:29
@JulianKniephoff
Copy link
Member

A few questions before we (finally ...) dive in:

@unterrichtsvideos
Copy link
Author

Comparing the commit history, the 3/3 branch (unterrichtsvideos:cc/master-merge-bugfixes-annotationsvorlagen---SQUASHED) was created from branch unterrichtsvideos:cc/master-merge-bugfixes---REBASED and the commit Fix missing translations, consistent language files (a6e0319), 4 non-conflicting commits were later added to the bugfixes branch. Otherwise the following 4 commits have to be added / merged to the 3/3 branch as well.

  • Disallow VSCode history (plugin, native) and browser DevTools files
  • Cleanup comments, remove debug logs
  • Exclude Git patch files
  • Fix scrolling behaviour of category list
  • Some of the comments already made by @unterrichtsvideos seem to suggest changes. Will someone make those, still?

These comments were part of a quick review and parts I stumbled upon. If that's fine for you, feel free to make these changes if you agree. Feel free to make any changes in the code regarding to the goal to create the merged version.

  • Can you add an example questionnaire JSON thingy (along with the necessary categories) here for testing? Or multiple if you have them? I remember having access to some folder, but I can't seem to find it right now. 🙈

the public meeting contains a link to test files, don't hesitate to contact us, whether these are working (they should) or need and update.

This access level can be removed (this belonged to a draft course context feature which is not included in the merge)
Creates a local sort function instead of having the exact same code two times.
Got overlooked in the latest merge
…ugfixes-annotationsvorlagen---SQUASHED

# Conflicts:
#	frontend/package.json
#	opencast-backend/annotation-impl/src/main/java/org/opencast/annotation/impl/persistence/ExtendedAnnotationServiceJpaImpl.java
#	opencast-backend/annotation-impl/src/test/java/org/opencast/annotation/impl/ExtendedAnnotationServiceJpaImplTest.java
#	pom.xml
@Arnei Arnei changed the base branch from merge to master July 28, 2023 10:13
@Arnei Arnei marked this pull request as ready for review July 28, 2023 11:18
For now it's always black, which works with our predefined color set.
Still, it would be nice for it to work with all possible colors. This
adds a todo reminder and a suggestion on how this might be achieved in the future.
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.

Is there documentation on how to use questionnaires? Might be me being blind, but I can't find any in this PR. And since users must use a template to use questionnaires, this feature is unusable if you don't know what a template must look like.

@unterrichtsvideos
Copy link
Author

Is there documentation on how to use questionnaires? Might be me being blind, but I can't find any in this PR. And since users must use a template to use questionnaires, this feature is unusable if you don't know what a template must look like.

Indeed we owe a documentation for this. We will create and provide one when the merged version is finalized.

Following the discussion regarding the term "questionnaire", for end users the feature is called "annotation templates", as a questionnaire is basically a "annotation-template"-based form editor for creating MCA with a pre-defined structure (which can be used e.g. to create annotation tasks for students); internally the term was not refactored

It was planned that templates can be created with a "questionnaire" builder (template builder)

Related project (unfinished state): Questionnaire builder
Code: https://github.com/luniki/oat-questionnaire-creator
Test-Deploy: https://www.uni-muenster.de/Unterrichtsvideos/builder/

Removes Edit, Save and Cancel buttons from questionnaire
timestamps. Simple input fields do the job just as well
and are easier to use.
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.

I'd say this is good enough to merge. Still needs to documentation, and did not go through the entire code (it's simply too much), but as far as I can tell the annotation templates work, don't break anything and this PR even fixes some previously broken things!

We'll still want to wait on @JulianKniephoff 's opinion before merging though.

@JulianKniephoff JulianKniephoff merged commit 1e5d5ea into opencast:master Dec 13, 2023
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.

6 participants