Skip to content

Fix default value in extended Metadata#812

Closed
Arnei wants to merge 3 commits intoopencast:mainfrom
Arnei:fix-extendend-metadata-initial-value-always-false
Closed

Fix default value in extended Metadata#812
Arnei wants to merge 3 commits intoopencast:mainfrom
Arnei:fix-extendend-metadata-initial-value-always-false

Conversation

@Arnei
Copy link
Copy Markdown
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
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 4, 2024

This pull request is deployed at test.admin-interface.opencast.org/812/2025-01-29_16-25-47/ .
It might take a few minutes for it to become available.

@github-actions
Copy link
Copy Markdown
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.

Comment thread src/utils/resourceUtils.ts Outdated
if (!!metadataCatalog.fields && metadataCatalog.fields.length > 0) {
metadataCatalog.fields.forEach((field) => {
let value = false;
let value: string | string[] | boolean = field.value;
Copy link
Copy Markdown
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
Copy Markdown
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?

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has conflicts ☹
Please resolve those so we can review the pull request.
Thanks.

@owi92
Copy link
Copy Markdown
Contributor

owi92 commented Jan 31, 2025

Rebasing this apparently nullified your changes, so I think this can be closed.

@Arnei Arnei closed this Jan 31, 2025
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.

3 participants