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

Fix default value in extended Metadata #812

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Jul 4, 2024

The default value for metadata fields in the extended metadata tab was always false instead of the intended default value (usually ""). This patch fixes that.

The default value for metadata fields in the
extended metadata tab was always `false`
instead of the intended default value (usually `""`).
This patch fixes that.
@Arnei Arnei added the type:bug Something isn't working label Jul 4, 2024
Copy link
Contributor

github-actions bot commented Jul 4, 2024

This pull request is deployed at test.admin-interface.opencast.org/812/2024-07-04_08-13-29/ .
It might take a few minutes for it to become available.

Copy link
Contributor

github-actions bot commented Jul 4, 2024

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-812

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-812

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.

@@ -125,7 +125,7 @@ export const getInitialMetadataFieldValues = (
for (const metadataCatalog of extendedMetadata) {
if (!!metadataCatalog.fields && metadataCatalog.fields.length > 0) {
metadataCatalog.fields.forEach((field) => {
let value = false;
let value: string | string[] | boolean = field.value;
Copy link
Member

Choose a reason for hiding this comment

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

This still doesn't seem correct to me. According to the docs, you can configure the type of each field (boolean, text, …). This would convert a field of type text with the value false to a boolean, wouldn't it? At the same time, types like date are kept as string. That seems weird. But maybe I'm missing something?

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, I don't quite get it either. I only tried to fix the issue I introduced myself here, so I mainly just put the code back "as it was". Why we would need to convert strings to booleans, and not other types like dates, and only for the extended metadata is beyond me. On the other hand, the conversion-to-boolean code currently never runs anyway, as we don't accept default values for metadata fields in the first place. So maybe it is better to remove the code entirely and have it behave more like the code for the main metadata catalog?

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