✨ Dynamic assess button and view assessments page #1325

ibolton336 · 2023-08-31T17:16:26Z

Moves QuestionsTable and AnswersTable components to shared root components directory
Adds assessment summary page - this page is reached via the "View" button on the assessment actions page. It shows the current assessment's answers by using the shared "QuestionnaireSummary" component which is also used by the questionnaire page referenced by assessment-settings-page in the admin view. The QuestionnaireSummary covers both cases with a conditional option for showing the answer key.
Adds Dynamic assessment actions button for view/take/retake/continue actions
Adds delete assessment button and functionality.
Add table for archived questionnaires in assessment actions page

Note: Will need to uncomment these lines for testing https://github.com/konveyor/tackle2-ui/pull/1325/files#diff-b0560fce9e9111641c87e7daa6648f4d60fd0b620522ff84501ec5dadd7f4128R38-R51

codecov · 2023-08-31T19:33:34Z

Codecov Report

Patch coverage: 27.77% and project coverage change: -0.41% ⚠️

Comparison is base (f52d7e2) 42.69% compared to head (abd3144) 42.29%.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1325      +/-   ##
==========================================
- Coverage   42.69%   42.29%   -0.41%     
==========================================
  Files         136      137       +1     
  Lines        4223     4287      +64     
  Branches     1000     1005       +5     
==========================================
+ Hits         1803     1813      +10     
- Misses       2408     2462      +54     
  Partials       12       12

Flag	Coverage Δ
client	`42.29% <27.77%> (-0.41%)`	⬇️
server	`∅ <ø> (∅)`

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

Files Changed	Coverage Δ
...s/components/application-form/application-form.tsx	`59.31% <0.00%> (-0.30%)`	⬇️
client/src/app/queries/assessments.ts	`20.68% <23.80%> (+1.17%)`	⬆️
client/src/app/queries/questionnaires.ts	`21.42% <25.00%> (ø)`
client/src/app/api/rest.ts	`55.05% <40.00%> (-0.35%)`	⬇️
client/src/app/Paths.ts	`91.11% <100.00%> (-8.89%)`	⬇️

... and 2 files with indirect coverage changes

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

mturley · 2023-09-07T13:39:41Z

client/src/mocks/stub-new-work/applications.ts

-  // rest.get(AppRest.APPLICATIONS, (req, res, ctx) => {
-  //   return res(ctx.json(mockApplicationArray));
-  // }),
+  // Commented out to avoid conflict with the real API


Did you mean to commit these uncommented or comment them back out?

Fixed. Thanks.

mturley · 2023-09-07T13:43:29Z

...cation-assessment/components/application-assessment-wizard/application-assessment-wizard.tsx

+        // const commentValues = values["comments"];
+        // const fieldName = getCommentFieldName(category, false);
+        // const commentValue = commentValues[fieldName];


Should we remove the commented out code here?

I'm not sure what we are doing about comments since they are yet another thing not available on the new API. Wa just going to leave them for now.

I see, ok. I wonder if we should leave a TODO then.

mturley

Mostly LGTM, just a couple things.

mturley · 2023-09-07T14:10:27Z

...cation-assessment/components/application-assessment-wizard/application-assessment-wizard.tsx

+        status: "STARTED",
+      };
+
+      await patchAssessment(payload);


I wonder if we want to switch to using a useMutation here eventually for these patchAssessment calls so we can separate API logic from the component. Sounds out of scope for this PR though since it's a preexisting pattern.

mturley · 2023-09-07T14:15:02Z

...pplications/application-assessment/components/assessment-summary/assessment-summary-page.tsx

+const AssessmentSummaryPage: React.FC = () => {
+  const { assessmentId } = useParams<AssessmentSummaryRouteParams>();
+
+  const { t } = useTranslation();
+
+  const [activeSectionIndex, setActiveSectionIndex] = React.useState<
+    "all" | number
+  >("all");
+
+  const handleTabClick = (
+    _event: React.MouseEvent<any> | React.KeyboardEvent | MouseEvent,
+    tabKey: string | number
+  ) => {
+    setActiveSectionIndex(tabKey as "all" | number);
+  };


Can we create a component shared by AssessmentSummaryPage and QuestionnairePage for the layout of tabs and search logic instead of copying that code here? I think maybe there's a higher level of stuff we can reuse here than just the QuestionsTable. We could create something like a QuestionnaireSummary that can handle either the questionnaire itself or an assessment with conditional logic for the parts that differ.

If that's too much hassle for this PR we could do it in a followup.

mturley · 2023-09-07T14:16:34Z

...tion-assessment/components/assessment-summary/components/questionnaire-section-tab-title.tsx

+import spacing from "@patternfly/react-styles/css/utilities/Spacing/spacing";
+import { Question } from "@app/api/models";
+
+const QuestionnaireSectionTabTitle: React.FC<{


Yeah, this component is straight up copied from the other page, I bet it could exist in one place and be used in the shared component.

mturley · 2023-09-07T14:22:33Z

client/src/app/pages/applications/assessment-actions/components/dynamic-assessment-button.tsx

+  const determineButtonBackgroundColor = () => {
+    const action = determineAction();
+    if (action === AssessmentAction.Continue) {
+      return "#5ba353"; // Continue background color
+    } else if (action === AssessmentAction.Retake) {
+      return "#F0AB01"; // Retake background color
+    } else {
+      return ""; // Default background color for "Take"
+    }
+  };


I wonder if instead we should conditionally apply CSS classNames to the buttons and define these colors in CSS?

Also we could use the CSS variables from the PF color palette instead of hard coded hash values. That way if we ever introduce dark mode the colors should be able to adjust automatically.

mturley

Looks awesome!

mturley · 2023-09-11T16:49:00Z

client/src/app/components/questionnaire-summary/questionnaire-summary.tsx

+          <div
+            style={{
+              backgroundColor: "var(--pf-v5-global--BackgroundColor--100)",
+            }}
+          >


At some point we might want to see if we can get rid of this inline style somehow

mturley · 2023-09-11T16:54:02Z

client/src/app/pages/applications/application-assessment/application-assessment.tsx


 export const ApplicationAssessment: React.FC = () => {
  const { t } = useTranslation();

  const { assessmentId } = useParams<AssessmentRoute>();
  const { assessment, isFetching, fetchError } =
-    useFetchAssessmentByID(assessmentId);
+    useFetchAssessmentById(assessmentId);


Signed-off-by: ibolton336 <ibolton@redhat.com> Take/retake flow for assessments Share questions and answers tables Hide answer key in assessment review screen Navigate back to assessment actions after assessment Signed-off-by: ibolton336 <ibolton@redhat.com> Separate out archived questionnaires Add continue button logic Add delete assessment mock and functionality Move shared questionnaire data Signed-off-by: ibolton336 <ibolton@redhat.com> Add comments Signed-off-by: ibolton336 <ibolton@redhat.com> Use RQ for patchAssessment Signed-off-by: ibolton336 <ibolton@redhat.com> Refactor assessment / questionnaire summary Signed-off-by: ibolton336 <ibolton@redhat.com> wire up delete for mock apps Signed-off-by: ibolton336 <ibolton@redhat.com> cleanup Signed-off-by: ibolton336 <ibolton@redhat.com> Fix invalidate queries after assessment patch Signed-off-by: ibolton336 <ibolton@redhat.com> Fix css Signed-off-by: ibolton336 <ibolton@redhat.com> Updates to match api Signed-off-by: ibolton336 <ibolton@redhat.com> Match hub status with api Signed-off-by: ibolton336 <ibolton@redhat.com> Test question updates Signed-off-by: ibolton336 <ibolton@redhat.com> Fix questionnaire by ID Signed-off-by: ibolton336 <ibolton@redhat.com> update tests Signed-off-by: ibolton336 <ibolton@redhat.com>

ibolton336 force-pushed the dynamic-assess-button branch from 1514222 to 68133cc Compare August 31, 2023 18:32

ibolton336 force-pushed the dynamic-assess-button branch from 28bed3c to a66ca60 Compare September 5, 2023 18:46

ibolton336 changed the title ~~[WIP] Dynamic assess button~~ ✨ Dynamic assess button and view assessments page Sep 5, 2023

ibolton336 force-pushed the dynamic-assess-button branch from 2114e86 to 6944d47 Compare September 5, 2023 20:32

ibolton336 marked this pull request as ready for review September 5, 2023 20:33

ibolton336 force-pushed the dynamic-assess-button branch from 6944d47 to dd8b2f4 Compare September 5, 2023 20:34

ibolton336 requested review from mturley and gildub September 5, 2023 20:34

ibolton336 force-pushed the dynamic-assess-button branch 2 times, most recently from e0527b2 to 88c5234 Compare September 5, 2023 20:41

ibolton336 marked this pull request as draft September 6, 2023 14:59

ibolton336 force-pushed the dynamic-assess-button branch 2 times, most recently from fa44853 to e9899cd Compare September 6, 2023 20:55

ibolton336 requested a review from sjd78 September 6, 2023 20:56

ibolton336 marked this pull request as ready for review September 6, 2023 20:56

mturley reviewed Sep 7, 2023

View reviewed changes

ibolton336 force-pushed the dynamic-assess-button branch from e9899cd to f066ed2 Compare September 7, 2023 13:43

mturley reviewed Sep 7, 2023

View reviewed changes

ibolton336 marked this pull request as draft September 8, 2023 16:07

ibolton336 force-pushed the dynamic-assess-button branch from 3186ed3 to abd3144 Compare September 8, 2023 17:11

ibolton336 marked this pull request as ready for review September 8, 2023 19:47

ibolton336 force-pushed the dynamic-assess-button branch from a32cefd to b69ce33 Compare September 8, 2023 19:47

mturley approved these changes Sep 11, 2023

View reviewed changes

ibolton336 force-pushed the dynamic-assess-button branch 4 times, most recently from 8c87d63 to a481e0b Compare September 11, 2023 17:30

ibolton336 force-pushed the dynamic-assess-button branch from a481e0b to b3c085b Compare September 11, 2023 17:42

ibolton336 merged commit e3f1630 into konveyor:main Sep 11, 2023
3 checks passed

ibolton336 deleted the dynamic-assess-button branch September 11, 2023 17:51

This was referenced Nov 22, 2023

operator konveyor-operator (0.3.0-beta.3) k8s-operatorhub/community-operators#3570

Merged

operator konveyor-operator (0.3.0-beta.3) redhat-openshift-ecosystem/community-operators-prod#3652

Merged

This was referenced Jan 23, 2024

operator konveyor-operator (0.3.0) k8s-operatorhub/community-operators#3793

Merged

operator konveyor-operator (0.3.0) redhat-openshift-ecosystem/community-operators-prod#3927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Dynamic assess button and view assessments page #1325

✨ Dynamic assess button and view assessments page #1325

ibolton336 commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading

mturley Sep 7, 2023

ibolton336 Sep 7, 2023

mturley Sep 7, 2023

ibolton336 Sep 7, 2023

mturley Sep 7, 2023

mturley left a comment

mturley Sep 7, 2023

ibolton336 Sep 7, 2023

mturley Sep 7, 2023

ibolton336 Sep 7, 2023

mturley Sep 7, 2023

ibolton336 Sep 7, 2023

mturley Sep 7, 2023

mturley left a comment

mturley Sep 11, 2023

mturley Sep 11, 2023

✨ Dynamic assess button and view assessments page #1325

✨ Dynamic assess button and view assessments page #1325

Conversation

ibolton336 commented Aug 31, 2023 • edited Loading

codecov bot commented Aug 31, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mturley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mturley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibolton336 commented Aug 31, 2023 •

edited

Loading

codecov bot commented Aug 31, 2023 •

edited

Loading