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

Undeprecate resources #1438

Merged
merged 48 commits into from
Dec 14, 2023
Merged

Undeprecate resources #1438

merged 48 commits into from
Dec 14, 2023

Conversation

danburonline
Copy link
Contributor

@danburonline danburonline commented Nov 27, 2023

Fixes #4509

Description

Major things

  • Added an undo deprecate button to the ResourceView component. When clicking on it it updates the states and goes to the latest revision (it's the same flow as when you, e.g. deprecate a resource).
    deprecated-undo
  • Don't show the undo deprecation button if it's an older resource revision.
  • Added a toast/notification in case the un-deprecation failed
    image
  • Don't allow undoing deprecation for unsupported resource types. Example:
    CleanShot 2023-11-30 at 11  06 15. This would need to be changed as soon as we allow the deprecation of other resource types as well.
  • Hide the button to deprecate the default Elasticsearch view, because it would break the listing operations (as described by @imsdu):
    no-deprecate-button

Cleaning

  • Sorted the package.json via sort-package-json => following common rules.
    • Unit tests failed after my adjustments as the QueryClient was missing. This is fixed.
  • Sorted imports of files I worked at. The ESLint rule is still missing (or generally ESLint).
  • Replaced the bitmap logo with an SVG.
  • Worked on reducing the delta from design to code by adjusting the logo size and the main white spaces on the pages. See the screenshot below for a before and after:

before-after

Additional Tickets Uncovered

  • 4541: The test coverage fell due to my adjustments. But to add new tests for the ResourceViewContainer component. This was out of scope for the the current ticket of this PR (#4509)
  • 4542: During the development I realised that the UI is quite messy, as it potentially grew organically (Frankenstein UI). This new ticket discusses a potential non-functional redesign.
  • 4543: During the development, I realised that the copy of the app was not consistent and, in general, maybe sometimes too technical/not conforming the docs and/or the mental model of Nexus. This new ticket discusses this.

How has this been tested?

  • Current tests pass.
  • Tested the flows locally via prod build.
  • I added an E2E test that tests the happy-path flow of deprecating a deprecatable resource. If I should add more E2E tests, let me know. I think the other non-happy paths can be tested via unit tests (as mentioned in 4541).

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have updated the documentation accordingly.
  • I have added necessary unit and integration tests.
  • I have added screenshots (if applicable), in the comment section.

@danburonline danburonline self-assigned this Nov 27, 2023
@danburonline danburonline changed the base branch from main to feature/org-welcome-page-bg November 27, 2023 08:50
@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2023

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (de04188) 48.08% compared to head (cfa3fdc) 48.05%.
Report is 1 commits behind head on develop.

Files Patch % Lines
src/shared/containers/ResourceViewContainer.tsx 66.03% 18 Missing ⚠️
src/shared/containers/ResourceActionsContainer.tsx 77.77% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1438      +/-   ##
===========================================
- Coverage    48.08%   48.05%   -0.04%     
===========================================
  Files          249      249              
  Lines        11368    11390      +22     
  Branches      2655     2665      +10     
===========================================
+ Hits          5466     5473       +7     
- Misses        5869     5883      +14     
- Partials        33       34       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@danburonline danburonline marked this pull request as ready for review November 30, 2023 15:26
Copy link
Contributor

@bilalesi bilalesi left a comment

Choose a reason for hiding this comment

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

good work, left some comments

regarding this part:
BlueBrain/nexus#4542: During the development I realised that the UI is quite messy, as it potentially grew organically (Frankenstein UI). This new ticket discusses a potential non-functional redesign.

There is already a design for the Resource details view and we already have a ticket for that
I will share it with you when we get there

@danburonline
Copy link
Contributor Author

@bilalesi I see that the tests are still failing (or sometimes fail and sometimes not). Will investigate before merging this.

src/shared/containers/ResourceViewContainer.tsx Outdated Show resolved Hide resolved
cypress/e2e/Resources.cy.ts Outdated Show resolved Hide resolved
Base automatically changed from feature/org-welcome-page-bg to develop December 8, 2023 14:23
As discussed today: Removing this
as we either way want to potentially
move to Playwright.
… feat/undeprecate-resource

# Conflicts:
#	package.json
@danburonline danburonline merged commit b3ae4dd into develop Dec 14, 2023
1 check passed
@danburonline danburonline deleted the feat/undeprecate-resource branch December 14, 2023 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants