Skip to content

Revert PR #1043 "Update react-router to 7.1.1"#1083

Closed
Arnei wants to merge 5 commits intoopencast:mainfrom
Arnei:revert-react-router-7.1.1
Closed

Revert PR #1043 "Update react-router to 7.1.1"#1083
Arnei wants to merge 5 commits intoopencast:mainfrom
Arnei:revert-react-router-7.1.1

Conversation

@Arnei
Copy link
Copy Markdown
Member

@Arnei Arnei commented Jan 23, 2025

Reverts PR #1043, specifically commit a4097a5. The PR seems to have caused problems with table filtering, which are fixed by reverting it. The issue in question is that clicking on a series name in the series table will not result in a properly filtered events table anymore. See also #1043 (comment)

How to test this

Can be tested as usual. Make sure to run npm install.

Reverts PR opencast#1043. The PR seems to have caused
problems with table filtering, which are fixed by
reverting it. The issue in question is that clicking
on a series name in the series table will not result
in a properly filtered events table anymore.
@Arnei Arnei added type:bug Something isn't working type:dependencies Pull requests that update a dependency file labels Jan 23, 2025
@github-actions
Copy link
Copy Markdown
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-1083

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

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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 23, 2025

This pull request is deployed at test.admin-interface.opencast.org/1083/2025-02-11_09-58-48/ .
It might take a few minutes for it to become available.

@Arnei Arnei changed the title Revert a4097a5f84c8d67d9774a55f4c73247d900e36d2 Revert PR #1043 "Update react-router to 7.1.1" Jan 28, 2025
@github-actions
Copy link
Copy Markdown
Contributor

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

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 4, 2025

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

@owi92
Copy link
Copy Markdown
Contributor

owi92 commented Feb 7, 2025

This doesn't seem to be working for me. The filter only gets applied sometimes, and when it does, it still takes a few seconds to display the right events.

Bildschirmaufnahme.2025-02-07.um.16.05.56.mov

And just as a side note for anyone else testing this branch, I had to take some extra steps before I could do so:
- before running npm install, I had to run rm -rf node_modules package-lock.json. Without doing that first, the app wouldn't start, because it Cannot find module @rollup/rollup-darwin-arm64. This seems to be related to npm/cli#4828.
- After doing rm -rf node_modules package-lock.json followed by another round of npm install, the app still didn't start correctly, informing me in devtools that there is an Uncaught Error: Could not resolve "@emotion/styled" imported by "@mui/styled-engine". Is it installed?
- so I also had to run npm install @emotion/styled @emotion/react
- with that, the app will start

@Arnei
Copy link
Copy Markdown
Member Author

Arnei commented Feb 10, 2025

Try as I might, I cannot reproduce your flukes. Is there any additional information to be had (in the webconsole etc.)?

Filters not being applied instantly is arguably a different issue.

@owi92
Copy link
Copy Markdown
Contributor

owi92 commented Feb 10, 2025

I just tried again, with the same outcome. There are no failed requests or errors in the console. I am on Node.js v20.18.2, if that is of any importance.
It's probably best if another person tests this, and if they don't have any issues, I think we can ignore mine. Just reverting that commit theoretically shouldn't break anything that wasn't broken before.

Also agree that filters not being applied instantly is another issue, it's just something that becomes apparent when testing these.

@github-actions
Copy link
Copy Markdown
Contributor

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

@Arnei Arnei force-pushed the revert-react-router-7.1.1 branch from 8248cc1 to 673b26e Compare February 11, 2025 09:58
Arnei added a commit to Arnei/opencast-admin-interface that referenced this pull request Feb 17, 2025
Includes opencast#980, supersedes  opencast#1083.

Should achieve the same thing as opencast#1083, but without
reverting to older dependency versions. Meaning
clicking on a series name in the series table
should bring up the events table with the series filter set.
Including opencast#980 as it helps in achieving this.
Arnei added a commit to Arnei/opencast-admin-interface that referenced this pull request Feb 21, 2025
Includes opencast#980, supersedes  opencast#1083.

Should achieve the same thing as opencast#1083, but without
reverting to older dependency versions. Meaning
clicking on a series name in the series table
should bring up the events table with the series filter set.
Including opencast#980 as it helps in achieving this.
@Arnei
Copy link
Copy Markdown
Member Author

Arnei commented Feb 26, 2025

Closing this in favor #1111.

@Arnei Arnei closed this Feb 26, 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 type:dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants