-
Notifications
You must be signed in to change notification settings - Fork 0
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
manage participants screen improvements #584
manage participants screen improvements #584
Conversation
|
||
export async function down(knex: Knex): Promise<void> { | ||
await knex.schema.alterTable('participants', (table) => { | ||
table.enu('status', ['initialize', 'awaitingApproval', 'approved']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should i leave this as is which sets all values to null or should i set all rows to 'approved'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably set them all to approved. I don't think the UI is expecting a null approval status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
const result = participants.sort((a, b) => a.name.localeCompare(b.name)); | ||
return res.status(200).json(result); | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed and moved over from participantsApproval.ts
import { SiteDTO } from '../../../api/services/adminServiceHelpers'; | ||
import { HighlightedResult } from './ParticipantApprovalForm'; | ||
import { SiteDTO } from '../../../../api/services/adminServiceHelpers'; | ||
import { HighlightedResult } from './HighlightedResult'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why this component was housed in ParticipantApprovalForm
but moved it to its own file since we are deleting ParticipantApprovalForm
anyways
src/web/services/participant.ts
Outdated
@@ -283,20 +266,3 @@ export type ParticipantApprovalFormDetails = { | |||
apiRoles: number[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this too? ParticipantApprovalFormDetails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah nice catch!
|
||
export async function down(knex: Knex): Promise<void> { | ||
await knex.schema.alterTable('participants', (table) => { | ||
table.enu('status', ['initialize', 'awaitingApproval', 'approved']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably set them all to approved. I don't think the UI is expecting a null approval status.
}; | ||
|
||
type HighlightedResultProps = Readonly<{ | ||
result: Fuse.FuseResult<SiteDTO>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This core component shouldn't have a dependency on a specific data type. Should be able to make this generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generalized it
What Changed:
dbo.participants
since all participants were defaulting to approved nowawaitingApproval
statusParticipantStatus
getApprovedParticipants
togetAllParticipants
HighlightedResult
into its own component fileApprovedParticipantsTable
toParticipantManagementTable
AuditTrailTable
)Test Plan: