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

✨ Convert assessment table to new table & begin assessment flow changes #1294

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

ibolton336
Copy link
Member

@ibolton336 ibolton336 commented Aug 17, 2023

  • Update questionnaire TS model to match hub proposal
type Questionnaire struct {
	Resource
	Name         string
	Description  string
	Revision     uint
	Required     bool
	Sections     []Section
	Thresholds   Thresholds
	RiskMessages RiskMessages
}

type Assessment struct {
	Resource
	Application   *Ref
	Archetype     *Ref
	Questionnaire *Ref
	Sections      []Section
}

@ibolton336 ibolton336 force-pushed the new-assessment-page branch from cd8e246 to 7746f90 Compare August 17, 2023 20:58
@codecov
Copy link

codecov bot commented Aug 17, 2023

Codecov Report

Patch coverage: 46.15% and no project coverage change.

Comparison is base (8ce3163) 42.96% compared to head (65d949d) 42.97%.

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

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1294   +/-   ##
=======================================
  Coverage   42.96%   42.97%           
=======================================
  Files         144      145    +1     
  Lines        4317     4330   +13     
  Branches      999      999           
=======================================
+ Hits         1855     1861    +6     
- Misses       2381     2457   +76     
+ Partials       81       12   -69     
Flag Coverage Δ
client 42.97% <46.15%> (+<0.01%) ⬆️
server ∅ <ø> (∅)

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

Files Changed Coverage Δ
client/src/app/queries/applications.ts 46.77% <36.36%> (-2.25%) ⬇️
client/src/app/Paths.ts 90.69% <100.00%> (+0.22%) ⬆️
client/src/app/data/mock-questionnaire.ts 100.00% <100.00%> (ø)

... and 29 files with indirect coverage changes

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

@ibolton336 ibolton336 force-pushed the new-assessment-page branch 3 times, most recently from 4e356ab to 6ca3661 Compare August 23, 2023 14:48
@ibolton336 ibolton336 marked this pull request as ready for review August 23, 2023 14:48
@mturley
Copy link
Collaborator

mturley commented Aug 23, 2023

Looks like there are some test failures due to mock imports

@ibolton336 ibolton336 force-pushed the new-assessment-page branch 3 times, most recently from 917b118 to 0fea180 Compare August 23, 2023 15:55
@ibolton336 ibolton336 requested a review from mturley August 23, 2023 17:51
Copy link
Collaborator

@mturley mturley left a comment

Choose a reason for hiding this comment

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

This is real good stuff @ibolton336 ! I just have a couple of small things we can improve.

Comment on lines +630 to +632
<NoDataEmptyState
title={t("composed.noDataStateTitle", {
what: t("terms.applications").toLowerCase(),
})}
description={t("composed.noDataStateBody", {
what: t("terms.application").toLowerCase(),
})}
/>
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

For some reason I am seeing this empty state on first page load just because the apps haven't been loaded yet. I'm surprised because you do already handle the loading case above with <ConditionalRender>, so there's something funky going on there.

Edit: Hm, it's also a problem on main actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seeing the empty state flash briefly before rendering the table items after removing the if statement mentioned above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm ok. Maybe we address that as a followup issue? However you want to handle that is fine with me

client/src/app/queries/applications.ts Show resolved Hide resolved
client/src/app/queries/applications.ts Show resolved Hide resolved
Update questionnaire model to match hub proposal

Start on Assessment Actions page

Restore drawer click bug fix

Add empty state for assessments

Update the dropdown within the app toolbar to remove old options

Fix kebab toggle styles for PF5

Add back in review functionality

Add pending confirmation dialog for assessment action

Setup mock data for assessment actions

Add actions table for assessment actions page

Signed-off-by: ibolton336 <[email protected]>
@ibolton336 ibolton336 force-pushed the new-assessment-page branch from 65d949d to e4ad6a5 Compare August 23, 2023 20:41
Copy link
Collaborator

@mturley mturley left a comment

Choose a reason for hiding this comment

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

LGTM!

@ibolton336 ibolton336 merged commit c763fc3 into konveyor:main Aug 24, 2023
@ibolton336 ibolton336 deleted the new-assessment-page branch August 24, 2023 17:46
ibolton336 added a commit to ibolton336/tackle2-ui that referenced this pull request Aug 24, 2023
…es (konveyor#1294)

- Update questionnaire TS model to match hub proposal

```
type Questionnaire struct {
	Resource
	Name         string
	Description  string
	Revision     uint
	Required     bool
	Sections     []Section
	Thresholds   Thresholds
	RiskMessages RiskMessages
}

type Assessment struct {
	Resource
	Application   *Ref
	Archetype     *Ref
	Questionnaire *Ref
	Sections      []Section
}

```
- Convert assessment table to use new table format
- No longer allow bulk assessment on assessment page top level toolbar
as this functionality is moving to Archetypes. Review & assessment
buttons moved to dropdown kebabs on the row level.
- Assessment Actions page with application level
questionnaires/assessments listed. TODO: [Add dynamic actions
button](konveyor#1299) based on
current assessment status
- Adds placeholder assessment modal when assessment action is triggered
from the assessment table row. TODO: [Add existing archetype
check](konveyor#1298)
- TODO: [View assessments page for existing
assessments](konveyor#1301) for
viewing existing assessments when clicking assess on an application that
already has an associated completed assessment
- TODO: [Convert assessment wizard
](konveyor#1306)
-TODO: Fix empty state initial render
konveyor#1311

Signed-off-by: ibolton336 <[email protected]>
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