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

feat(RHIF-255): Add Zero State to image builder #1231

Closed
wants to merge 1 commit into from

Conversation

adonispuente
Copy link

@adonispuente adonispuente commented Jul 11, 2023

This PR adds zero state to image builder.
To test use an account with zero systems, or change the logic for what the state is supposed to show and verify things look correct.

This zero state should pop up when there are 0 images on the account.

Fixes https://issues.redhat.com/browse/HMS-2184

@codecov
Copy link

codecov bot commented Jul 11, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (44e2a95) 73.30% compared to head (dbe9748) 73.59%.
Report is 193 commits behind head on main.

❗ Current head dbe9748 differs from pull request most recent head a7257eb. Consider uploading reports for the commit a7257eb to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1231      +/-   ##
==========================================
+ Coverage   73.30%   73.59%   +0.29%     
==========================================
  Files          70       69       -1     
  Lines        2000     1992       -8     
  Branches      529      527       -2     
==========================================
  Hits         1466     1466              
+ Misses        483      475       -8     
  Partials       51       51              

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44e2a95...a7257eb. Read the comment docs.

@adonispuente adonispuente marked this pull request as ready for review July 12, 2023 19:00
@croissanne
Copy link
Member

croissanne commented Jul 13, 2023

Thank you! A few thoughts:

  1. Now that edge is also integrating, I think we should figure out how those two are supposed to play together.
  2. I wonder if we should show it when linking directly to the wizard
  3. The beta prompt/popup is no longer visible if there's no images

@arburka @kelseamann would you mind taking a look as well?

@croissanne croissanne marked this pull request as draft July 18, 2023 15:41
@arburka
Copy link

arburka commented Jul 18, 2023

Edge integration should not affect when this screen is shown. The zero state is shown when the "star" box in this flow happens (link)

If we link directly to the wizard, this should/could still show below it if the logic is sound. The button on the page evokes the wizard as well.

Yes, the beta prompt will not be visible. This is ok as we are close to removing them for these features and we would want to show that content in a similar way if we plan to highlight it here.

For example, we would want to add custom content to this page.

src/Router.js Outdated Show resolved Hide resolved
src/Router.js Outdated Show resolved Hide resolved
@lucasgarfield
Copy link
Collaborator

Hi @adonispuente, would you mind squashing the second commit into the first one?

@lavocatt
Copy link
Contributor

It would be nice to wait up until #1320 is merged so that we can use the new hook to know how many images are in the images table.

@lavocatt lavocatt force-pushed the RHIF-255 branch 4 times, most recently from ad8ad71 to a866172 Compare September 20, 2023 12:44
@lavocatt lavocatt self-assigned this Sep 20, 2023
@lavocatt lavocatt marked this pull request as ready for review September 20, 2023 12:46
@lavocatt lavocatt force-pushed the RHIF-255 branch 2 times, most recently from 6d9bc2b to 23b5c75 Compare September 20, 2023 14:08
src/Router.js Outdated Show resolved Hide resolved
src/Router.js Outdated Show resolved Hide resolved
src/Router.tsx Outdated Show resolved Hide resolved
@lavocatt
Copy link
Contributor

lavocatt commented Oct 11, 2023

The unit tests are fixed @jrusz all good for you to test it.

@jrusz
Copy link
Collaborator

jrusz commented Oct 11, 2023

The unit tests are fixed @jrusz all good for you test it.

Thanks! I'll get on it.

@jrusz
Copy link
Collaborator

jrusz commented Oct 11, 2023

/retest

2 similar comments
@jrusz
Copy link
Collaborator

jrusz commented Oct 11, 2023

/retest

@jrusz
Copy link
Collaborator

jrusz commented Oct 11, 2023

/retest

@jrusz
Copy link
Collaborator

jrusz commented Oct 11, 2023

Lol it actually passed 😄 ... Since I know already where to update the ouiaID I'm going to do that and then change it a little bit to make it more stable.

Copy link
Contributor

@lavocatt lavocatt left a comment

Choose a reason for hiding this comment

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

Quoting @croissanne:

Theoretically there are 3 "empty" states: an empty page state (returning user who's images expired), the zeroState, and the unentitled page.The unentitled page is of no concern to us because as soon as you have RHEL you're entitled. The empty page state will go away with blueprints (since those don't expire). Meaning after blueprints we can do the new zerostate.

There's no good ways to distinguish between the 3 states rn, so I'm blocking this work up until the blueprints wizard is there.

Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Nov 13, 2023
@lavocatt lavocatt removed the Stale label Nov 13, 2023
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added Stale and removed Stale labels Dec 14, 2023
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 15, 2024
@lavocatt lavocatt removed the Stale label Jan 15, 2024
@app-sre-bot
Copy link
Collaborator

Can one of the admins verify this patch?

Copy link
Contributor

github-actions bot commented Mar 8, 2024

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 8, 2024
@regexowl regexowl removed the Stale label Mar 14, 2024
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label Apr 14, 2024
@lavocatt lavocatt removed their assignment Apr 16, 2024
@github-actions github-actions bot removed the Stale label Apr 17, 2024
Copy link
Contributor

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale label May 18, 2024
Copy link
Contributor

This PR was closed because it has been stalled for 30+7 days with no activity.

@github-actions github-actions bot closed this May 25, 2024
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.

None yet

8 participants