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

Unify Scheduling pages #911

Merged
merged 3 commits into from
Jan 21, 2025
Merged

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Sep 12, 2024

Fixes #221

Reduces duplicate code between the scheduling page for creating a new event and for editing events.
Functionality should be unchanged.

@Arnei Arnei added the type:code-enhancement Internal improvements to the codebase label Sep 12, 2024
Copy link
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/opencast-admin-interface:pr-911

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

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.

Reduces duplicate code between the scheduling page for
creating a new event and for editing events.
Functionality should be unchanged.
@Arnei Arnei force-pushed the unify-scheduling-fields branch from ebb3d24 to 998e31b Compare September 12, 2024 13:24
Copy link
Contributor

github-actions bot commented Sep 12, 2024

This pull request is deployed at test.admin-interface.opencast.org/911/2025-01-21_07-58-59/ .
It might take a few minutes for it to become available.

@snoesberger
Copy link
Contributor

I have done some tests with this PR and found some problems:

grafik

@Arnei
Copy link
Member Author

Arnei commented Nov 11, 2024

Thanks for testing!

Did the problems you found also occur without this PR? (I think I've seen the start date field being empty on another branch before, but I'm not sure)

This PR does not intend to change functionality. To fix #941, a seperate PR would be better. (Getting this merged should make fixing it easier though)

@snoesberger
Copy link
Contributor

Did the problems you found also occur without this PR? (I think I've seen the start date field being empty on another branch before, but I'm not sure)

I did another test with the actual main branch and I have the same problem there. But on our test system with the opencast-admin-interface release from 2024-07-12 installed, I don't have this problem. The date is displayed correctly there. So somehow this bug was introduced afterwards.

This PR does not intend to change functionality. To fix #941, a seperate PR would be better. (Getting this merged should make fixing it easier though)

Ok, I just wanted to make sure we don't forget about this issue (and don't reintroduce it later on).

Copy link
Member

Choose a reason for hiding this comment

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

Missing symbols:

> tsc && eslint . --max-warnings=0 && vite build

src/components/events/partials/ModalTabsAndPages/NewSourcePage.tsx:445:18 - error TS2304: Cannot find name 'currentLanguage'.

445          locale={currentLanguage?.dateLocale}
                     ~~~~~~~~~~~~~~~

src/components/events/partials/ModalTabsAndPages/NewSourcePage.tsx:475:20 - error TS2304: Cannot find name 'currentLanguage'.

475            locale={currentLanguage?.dateLocale}
                       ~~~~~~~~~~~~~~~


Found 2 errors in the same file, starting at: src/components/events/partials/ModalTabsAndPages/NewSourcePage.tsx:445

Copy link
Member Author

Choose a reason for hiding this comment

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

Can't reproduce this. The missing symbols are not present in the NewSourcePage.tsx?

Copy link
Member

Choose a reason for hiding this comment

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

They're not in the diff, they're in the origin/main. I'm guessing main drifted underneath this PR.

@gregorydlogan gregorydlogan merged commit 8c4abdc into opencast:main Jan 21, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:code-enhancement Internal improvements to the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate code for scheduling
3 participants