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

✨ Add assessed archetypes section in drawer #1610

Merged
merged 6 commits into from
Dec 14, 2023

Conversation

ibolton336
Copy link
Member

@ibolton336 ibolton336 commented Dec 11, 2023

Copy link

codecov bot commented Dec 11, 2023

Codecov Report

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

Comparison is base (3b5b3be) 39.13% compared to head (e820ff2) 39.18%.

Files Patch % Lines
client/src/app/components/IconedStatus.tsx 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1610      +/-   ##
==========================================
+ Coverage   39.13%   39.18%   +0.05%     
==========================================
  Files         146      146              
  Lines        4850     4856       +6     
  Branches     1205     1209       +4     
==========================================
+ Hits         1898     1903       +5     
- Misses       2850     2851       +1     
  Partials      102      102              
Flag Coverage Δ
client 39.18% <88.88%> (+0.05%) ⬆️
server ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

The view looks ok if the application does not have an archetype or if the archetype has a completed assessment or review. But if the archetype has assessments in progress, the view is broken

✔️ No archetype (looks good):
screenshot-localhost_9000-2023 12 12-16_59_25

✔️ Archetype with assessment and review completed:
screenshot-localhost_9000-2023 12 12-17_06_19

❌ Archetype but the assessment is not started of in-progress:
screenshot-localhost_9000-2023 12 12-17_02_18

So just need to have the 3rd screen there show "None" or similar

Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

Behavior for application with an archetype but the archetype is missing an assessment or review is looking good now. The "None" shows as expected.

Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

I'm not getting the inherited text where I saw the infotip icon before.

Address singular translation issue

Signed-off-by: ibolton336 <[email protected]>
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

Everything is looking good, just a question on the review column.

const { t } = useTranslation();

const { archetypes, isFetching } = useFetchArchetypes();
const isAppReviewed = !!application.review;
Copy link
Member

Choose a reason for hiding this comment

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

Is the review one for the application or does it include the archetype review? This column may have the same issue as the assessment column...

Copy link
Member Author

Choose a reason for hiding this comment

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

The review only includes the app level review. There is no review bool, this is just the actual review object on the application.

@sjd78 sjd78 changed the title ✨ Add assessed archetypes section in drawer: ✨ Add assessed archetypes section in drawer Dec 13, 2023
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

2 participants