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: show a full page 404 when a project is not accessible #3162

Merged
merged 15 commits into from
Jun 3, 2024

Conversation

leafty
Copy link
Member

@leafty leafty commented May 30, 2024

Details:

  • Add a "Error 404 - Project no found" page with contents similar to v1 not found page.
  • Refactor the project page v2 routing and components (see <Outlet> and useOutletContext())

Examples:

/deploy

@leafty leafty temporarily deployed to renku-ci-ui-3162 May 30, 2024 11:48 — with GitHub Actions Inactive
@RenkuBot
Copy link
Contributor

You can access the deployment of this PR at https://renku-ci-ui-3162.dev.renku.ch

@leafty leafty temporarily deployed to renku-ci-ui-3162 May 30, 2024 14:28 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-ui-3162 May 30, 2024 14:44 — with GitHub Actions Inactive
@leafty leafty marked this pull request as ready for review May 31, 2024 08:06
@leafty leafty requested a review from a team as a code owner May 31, 2024 08:06
@leafty leafty temporarily deployed to renku-ci-ui-3162 May 31, 2024 08:07 — with GitHub Actions Inactive
@leafty leafty temporarily deployed to renku-ci-ui-3162 May 31, 2024 08:43 — with GitHub Actions Inactive
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

praise: This code improvements here are very nice, and the use of Outlet allows for a better structure to the project page!

I found a situation were the new 404 page should be shown, but the old error message is still displayed: https://renku-ci-ui-3162.dev.renku.ch/v2/projects/foo/test/sessions/01HZ79337RV3CCZCQSMW8F0V5F/start

@leafty leafty temporarily deployed to renku-ci-ui-3162 May 31, 2024 13:00 — with GitHub Actions Inactive
@leafty
Copy link
Member Author

leafty commented May 31, 2024

praise: This code improvements here are very nice, and the use of Outlet allows for a better structure to the project page!

I found a situation were the new 404 page should be shown, but the old error message is still displayed: https://renku-ci-ui-3162.dev.renku.ch/v2/projects/foo/test/sessions/01HZ79337RV3CCZCQSMW8F0V5F/start

Fixed in 7e10153.

@leafty leafty requested a review from ciyer May 31, 2024 13:01
ciyer
ciyer previously approved these changes May 31, 2024
Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

Fix looks good!

@leafty leafty temporarily deployed to renku-ci-ui-3162 May 31, 2024 13:42 — with GitHub Actions Inactive
@leafty leafty requested a review from ciyer May 31, 2024 13:51
@leafty
Copy link
Member Author

leafty commented May 31, 2024

@ciyer can you re-approve? There was a merge conflict and also the "Delete" button redirected to nowhere (fixed).

Copy link
Contributor

@ciyer ciyer left a comment

Choose a reason for hiding this comment

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

LGTM!

@leafty leafty merged commit 215af16 into main Jun 3, 2024
18 checks passed
@leafty leafty deleted the leafty/full-page-404 branch June 3, 2024 08:05
@RenkuBot
Copy link
Contributor

RenkuBot commented Jun 3, 2024

Tearing down the temporary RenkuLab deplyoment for this PR.

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.

3 participants