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.

Sign up for GitHub

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

Jump to bottom

✨ Initial Assessment page #1268

Merged
merged 8 commits into from
Aug 9, 2023
Merged

✨ Initial Assessment page #1268

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
abf338a
Initial Assessment page
gildub Aug 9, 2023
224fa84
Add mock delete hook and some fixes
gildub Aug 9, 2023
7ea322f
Forget about unused params
gildub Aug 9, 2023
e67583c
Remove bulk selector
gildub Aug 9, 2023
71f8c1e
More comment and typo
gildub Aug 9, 2023
72137e6
Remove unused selectedItems
gildub Aug 9, 2023
bfa71c1
Rename page as AssessmentSettings
gildub Aug 9, 2023
c171cfc
Rename path to follow requirement
gildub Aug 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion client/public/locales/en/translation.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"delete": "Delete",
"discardAssessment": "Discard assessment/review",
"downloadCsvTemplate": "Download CSV template",
"download": "Download {{what}}",
"edit": "Edit",
"export": "Export",
"filterBy": "Filter by {{what}}",
Expand All @@ -47,6 +48,7 @@
"selectNone": "Select none",
"selectPage": "Select page",
"submitReview": "Submit review",
"view": "View",
"viewErrorReport": "View error report"
},
"colors": {
Expand Down Expand Up @@ -101,8 +103,11 @@
"copyApplicationAssessmentFrom": "Copy {{what}} assessment",
"delete": "Delete {{what}}?",
"discard": "Discard {{what}}?",
"download": "Download {{what}}",
"edit": "Edit {{what}}",
"export": "Export {{what}}",
"importApplicationFile": "Import application file",
"import": "Import {{what}}",
"leavePage": "Leave page",
"new": "New {{what}}",
"newApplication": "New application",
Expand Down Expand Up @@ -302,6 +307,8 @@
"proxyConfig": "Proxy configuration",
"proxyConfigDetails": "Manage connections to proxy servers",
"question": "Question",
"questionnaire": "Questionnaire",
"questionnaires": "Questionnaires",
"rank": "Rank",
"project": "Project",
"refresh": "Refresh",
Expand Down Expand Up @@ -356,7 +363,8 @@
"user": "User",
"version": "Version",
"workPriority": "Work priority",
"tag": "Tag"
"tag": "Tag",
"YAMLTemplate": "YAML template"
},
"toastr": {
"success": {
Expand Down
1 change: 1 addition & 0 deletions client/src/app/Paths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export enum Paths {
repositoriesMvn = "/repositories/maven",
proxies = "/proxies",
migrationTargets = "/migration-targets",
questionnaires = "/questionnaires",
jira = "/jira",
}

Expand Down
6 changes: 6 additions & 0 deletions client/src/app/Routes.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const AffectedApplications = lazy(
() => import("./pages/issues/affected-applications")
);
const Dependencies = lazy(() => import("./pages/dependencies"));
const Questionnaires = lazy(() => import("./pages/questionnaires"));

export interface IRoute {
path: string;
Expand Down Expand Up @@ -120,6 +121,11 @@ export const devRoutes: IRoute[] = [
},
]
: []),
{
path: Paths.questionnaires,
comp: Questionnaires,
exact: false,
},
];

export const adminRoutes: IRoute[] = [
Expand Down
14 changes: 12 additions & 2 deletions client/src/app/api/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,10 @@ export interface Assessment {
status: AssessmentStatus;
stakeholders?: number[];
stakeholderGroups?: number[];
questionnaire: Questionnaire;
questionnaire: PathfinderQuestionnaire;
}

export interface Questionnaire {
export interface PathfinderQuestionnaire {
categories: QuestionnaireCategory[];
}

Expand Down Expand Up @@ -704,3 +704,13 @@ export type HubFile = {
name: string;
path: string;
};

export interface Questionnaire {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we name this CustomQuestionnaire to distinguish it from PathfinderQuestionnaire rather than imply inheritance between the two? or is it intended that all questionnaires will eventually use this type and pathfinder objects will be deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mturley, that's a good idea although my understanding is that Pathfinder objects are going to become obsolete.
Let me get confirmation that from the Hub team.
The other thing is the custom questionnaire is a subset of questionnaire because there are system questionnaire provided by Tackle.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if pathfinder will be removed eventually and we'll only have one questionnaire type, i'm fine with leaving the name as-is.

id: number;
required: boolean;
name: string;
questions: number;
rating: string;
dateImported: string;
system: boolean;
}
15 changes: 15 additions & 0 deletions client/src/app/api/rest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
Rule,
Target,
HubFile,
Questionnaire,
} from "./models";
import { QueryKey } from "@tanstack/react-query";
import { serializeRequestParamsForHub } from "@app/hooks/table-controls";
Expand Down Expand Up @@ -106,6 +107,8 @@
export const ANALYSIS_ISSUE_INCIDENTS =
HUB + "/analyses/issues/:issueId/incidents";

export const QUESTIONNAIRES = HUB + "/questionnaires";

// PATHFINDER
export const PATHFINDER = "/hub/pathfinder";
export const ASSESSMENTS = PATHFINDER + "/assessments";
Expand Down Expand Up @@ -739,3 +742,15 @@

export const updateProxy = (obj: Proxy): Promise<Proxy> =>
axios.put(`${PROXIES}/${obj.id}`, obj);

// Questionnaires

export const getQuestionnaires = (): Promise<Questionnaire[]> =>
axios.get(QUESTIONNAIRES).then((response) => response.data);

Check warning on line 749 in client/src/app/api/rest.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/api/rest.ts#L749 

Added line #L749 was not covered by tests

export const updateQuestionnaire = (
obj: Questionnaire
): Promise<Questionnaire> => axios.put(`${QUESTIONNAIRES}/${obj.id}`, obj);

Check warning on line 753 in client/src/app/api/rest.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/api/rest.ts#L753 

Added line #L753 was not covered by tests

export const deleteQuestionnaire = (id: number): Promise<Questionnaire> =>
axios.delete(`${QUESTIONNAIRES}/${id}`);

Check warning on line 756 in client/src/app/api/rest.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/api/rest.ts#L756 

Added line #L756 was not covered by tests
8 changes: 8 additions & 0 deletions client/src/app/layout/SidebarApp/SidebarApp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,14 @@ export const SidebarApp: React.FC = () => {
</NavItem>
</NavExpandable>
) : null}
<NavItem>
<NavLink
to={Paths.questionnaires}
activeClassName="pf-m-current"
>
Assessment
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose this is design feedback for Justin but it seems confusing to me that we'll now have two views called Assessment - one here and one as a tab in application inventory. I guess because this is under admin it makes sense, but I wonder if we should propose that the nav item and page title are called "Questionnaires" instead, and a subtitle in the page header could be used to explain that they are for assessment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mturley,
My understanding is that the Administrator -> Assessment page is actually displaying the questionnaires but the Required button activate those questionnaires to the default Assessment process.
So the Questionnaires.tsx page might actually not the correct name. I wonder if I should rename it now or wait.
Because this page handles Assessment settings.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah maybe Assessment Settings is a better name than Assessment then. I just find the current name potentially confusing to users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ibolton336, what do you think about renaming that page Assessment.tsx ?
Because the used case is about providing Administrator a way to control assessment, which starts with activating (Required column) questionnaires.

Copy link
Member

Choose a reason for hiding this comment

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

I still find that to be a naming conflict. I'd rather it be something like AssessmentTools/QuestionaireSettings/AssessmentSettings etc.

</NavLink>
</NavItem>
</NavList>
)}
</Nav>
Expand Down
Loading
Loading