-
Notifications
You must be signed in to change notification settings - Fork 9
#3282 edit abbreviation table #3402
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
#3282 edit abbreviation table #3402
Conversation
Zwendle
left a comment
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.
Nit: change SetAbbreviationModel file and component name to SetAbbreviationModal
| /> | ||
| <Typography variant="subtitle1">Project Name Abbreviations</Typography> | ||
| <AdminToolTable columns={[{ name: 'Project Name' }, { name: 'Abbreviation' }]} rows={[]} /> | ||
| <AdminToolTable columns={[{ name: 'Project Name' }, { name: 'Abbreviation' }]} rows={projectTableRows} /> |
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.
Nit: After creating an abbreviation, the first cell in the right-most column doesn't have the same black border as the other cells. If you add a third { name: '' } to the columns prop that should fix the styling issue (not sure if that's the best way to fix it though lol)
| } | ||
|
|
||
| /** | ||
| * Revomves the abbreviation from a given project |
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.
typo
| const project = await ProjectsService.getSingleProjectWithQueryArgs(wbsNum, organization); | ||
|
|
||
| if (!(await userHasPermission(user.userId, organization.organizationId, isAdmin))) { | ||
| throw new AccessDeniedAdminOnlyException('delete Abbreviation'); |
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.
nit: don't capitalize here
| * @returns the updated project | ||
| */ | ||
| static async setAbbreviation(wbsNum: WbsNumber, user: User, organization: Organization, abbreviation: string) { | ||
| const project = await ProjectsService.getSingleProjectWithQueryArgs(wbsNum, organization); |
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.
Throw if the project isn't found
| * @returns the updated project | ||
| */ | ||
| static async deleteAbbreviation(wbsNum: WbsNumber, user: User, organization: Organization) { | ||
| const project = await ProjectsService.getSingleProjectWithQueryArgs(wbsNum, organization); |
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.
Same here
Changes
Added features to the abbreviation table to set abbreviations (existing and new), using set and delete endpoints
Test Cases
Screenshots
Postman testing not done because you can test it with the table (add something and then refresh)
Checklist
It can be helpful to check the
ChecksandFiles changedtabs.Please review the contributor guide and reach out to your Tech Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.
yarn.lockchanges (unless dependencies have changed)Closes #3282