-
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
GAP-2140: UI for view & manage editors #349
Conversation
21b4a42
to
5044511
Compare
function formatEditorRows({ email: key, role: value, id }: EditorList) { | ||
const editorRow: UnformattedEditorRow = { key, value }; | ||
if (isOwner) | ||
editorRow.action = { | ||
href: `/scheme/${schemeId}/manage-editors/remove/${id}`, | ||
label: 'Remove', | ||
ariaLabel: `Remove ${key}`, | ||
}; | ||
return editorRow; | ||
} | ||
|
||
const editorRows = ( | ||
await getSchemeEditors(schemeId, sessionCookie, userServiceJwt) | ||
).map(formatEditorRows); | ||
|
||
editorRows.unshift({ | ||
key: 'Email', | ||
value: 'Role', | ||
action: isOwner ? 'Actions' : '', | ||
} as UnformattedEditorRow); |
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.
In my opinion this logic should live client side, server should be reserved for data fetching & error handling. Client for determining how to render said data which this logic falls under
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.
There is also a lot of logic thats formatting the data for this table. We have the following functions:
- formatEditorRows
- formatEditorRow
and logic in between all that too:
const editorRows = schemeEditors.map(formatEditorRows);
editorRows.unshift({
key: 'Email',
value: 'Role',
action: isOwner ? 'Actions' : '',
});
const mappedTableRows = editorRows.reduce(formatEditorRow, []);
I'm finding it very hard to follow what its actually doing and I suspect this can be cleaned up
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.
Have had a go at cleaning up / extracting some of the logic. Let me know if you have any other ideas for it :)
export type EditorList = { | ||
role: 'EDITOR' | 'OWNER'; | ||
email: string; | ||
id: string; | ||
}; |
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.
Think this type should be shifted to the scheme service (and ideally we have an editor service)
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.
Sure, moved that
@@ -0,0 +1,10 @@ | |||
.breakLine { | |||
background-color: #1d70b8; |
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.
Fetch colours from https://design-system.service.gov.uk/styles/colour/ like so:
@import '~govuk-frontend/govuk/helpers/colour';
.gap-light-grey-background {
background-color: govuk-colour("light-grey");
}
not checked which colour this hex matches up to so make sure you get the right one 👀
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.
fixed
packages/admin/src/pages/scheme/[schemeId]/editors.getServerSideProps.tsx
Show resolved
Hide resolved
export const isSchemeOwner = async (schemeId: string, sessionId: string) => { | ||
const { data } = await axios.get<boolean>( | ||
`${SCHEME_EDITORS_HOST}/${schemeId}/isOwner`, | ||
axiosSessionConfig(sessionId) | ||
); | ||
return data; | ||
}; | ||
|
||
export const getSchemeEditors = async ( | ||
schemeId: string, | ||
sessionId: string, | ||
userServiceJwt: string | ||
) => { | ||
const { data } = await axios.get<EditorList[]>( | ||
`${SCHEME_EDITORS_HOST}/${schemeId}`, | ||
getFullConfig(sessionId, userServiceJwt) | ||
); | ||
return data; | ||
}; |
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.
Shift to SchemeEditorService
@@ -35,6 +35,13 @@ export const axiosSessionConfig = (sessionId: string) => { | |||
}; | |||
}; | |||
|
|||
export const getFullConfig = (sessionId: string, userServiceJwt: string) => ({ |
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.
Nice I rate this <3
const STYLE_MAP = { | ||
displayRegularKeyFont: 'gap-summary-list--key-weight-regular', | ||
hasWiderKeyColumn: 'key-width-45percent-sm', | ||
equalColumns: 'equal-columns', | ||
useBoldFont: 'fw-bold', | ||
actionTextLeft: 'text-left', | ||
}; |
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.
I miss tailwind
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 would be a lot cleaner with tailwind tbh
packages/admin/src/pages/scheme/[schemeId]/manage-editors/index.page.tsx
Outdated
Show resolved
Hide resolved
…x.page.tsx Co-authored-by: dominicwest <[email protected]> Signed-off-by: john-tco <[email protected]>
packages/admin/src/pages/scheme/[schemeId]/view-editors/index.page.tsx
Outdated
Show resolved
Hide resolved
UnformattedEditorRow, | ||
getServerSideProps, | ||
} from '../editors.getServerSideProps'; | ||
import { Row } from 'gap-web-ui/dist/cjs/components/summary-list/SummaryList'; |
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.
I reckon this might break in sandbox - need to export this type from gap-web-ui and import properly
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.
oops, good spot
…page.tsx Co-authored-by: dominicwest <[email protected]> Signed-off-by: john-tco <[email protected]>
* GAP-2140: UI for view & manage editors (#349) * initial * push content * add view-editors page * blank commit to trigger build * fix linting * fix unnamed test component * update routes * amend error redirect * rm * shift to editor service * use colour helper * add editors mock * fix editors gssp * Update packages/admin/src/pages/scheme/[schemeId]/manage-editors/index.page.tsx Co-authored-by: dominicwest <[email protected]> Signed-off-by: john-tco <[email protected]> * fix url * misc * refactor * update import * Update packages/admin/src/pages/scheme/[schemeId]/view-editors/index.page.tsx Co-authored-by: dominicwest <[email protected]> Signed-off-by: john-tco <[email protected]> --------- Signed-off-by: john-tco <[email protected]> Co-authored-by: dominicwest <[email protected]> * TMI2-689 - removed unnecessary styling from div (#357) * Fixing un-needed QuestionPageServerSideProps prop (#347) * GAP-2137 Add a new editor (#338) * TMI2-600: Download all applications / Error in export (#353) * TMI2-602, TMI2-603, TMI2-604, TMI2-617: Download all applications (#323) * TMI2-602 - Initial commit of updated page * TMI2-602 - Adding unit tests * TMI2-602 - Restructuring page to accommodate for all journeys * TMI2-603, TMI-604 - Adding error banner and placeholders for pagination * TMI2-603 - Tidying up --------- Co-authored-by: Jim Purvis <[email protected]> * TMI2-633: Functional 'Download all' button (#340) * TMI2-602 - Initial commit of updated page * TMI2-602 - Adding unit tests * TMI2-602 - Restructuring page to accommodate for all journeys * TMI2-603, TMI-604 - Adding error banner and placeholders for pagination * TMI2-603 - Tidying up * TMI2-633 - Adding functionality to Download All button * TMI2-633 - Removing console.log --------- Co-authored-by: Jim Purvis <[email protected]> * TMI2-603-download-individual-applications-success-path (#346) * TMI2-602 - Initial commit of updated page * TMI2-602 - Adding unit tests * TMI2-602 - Restructuring page to accommodate for all journeys * TMI2-603, TMI-604 - Adding error banner and placeholders for pagination * TMI2-603 - Tidying up * Add individual download page * Update pagination component * Add back button url --------- Co-authored-by: Jim Purvis <[email protected]> Co-authored-by: jimpurvisTCO <[email protected]> * TMI2-600 restructure (#351) * TMI2-600 - Restructuring and moving 'Download all submissions' page to meet new API spec * TMI2-600 - Connecting up to new export details API * TMI2-600 - Adding pagination (still to be linked to view page) --------- Co-authored-by: Jim Purvis <[email protected]> * TMI2-600 - Only requesting necessary exports for each * TMI2-600 - Unit tests * TMI2-603: Update download-individual page * Integrate API to get submissions * Implement pagination * Remove redundant code * TMI2-603: Add unit tests for download-individual page * TMI2-605 - View submission summary (error path) (#352) * TMI2-605 - View read-only summary of application that could not be downloaded * TMI2-605 - Added link to view individual read-only submissions * TMI2-605 - Linking frontend to new failed export details endpoint --------- Co-authored-by: Jim Purvis <[email protected]> * TMI2-600 - fix for use of an 'any' in submissions list * Visual tweaks * TMI2-600 - typo fix * Clean up code * TMI2-669 - download attachments error path (#356) * TMI2-669 - Added conditional logic for rendering option to download attachments * TMI2-669 - Added tests * Update DownloadMessage component * Component now refers to plural applications if there is more than 1 * Update download-individual page * Pagination now only displays margin when there is more than 1 page * Item type is now set to applications * TMI2-600 - PR comments re: view application * TMI2-600 - Unit test update * Update index.page.tsx and download-individual.page.tsx * Replace DownloadMessage banner with ImportantBanner * Align text with Figma designs * Make pagination more readable * Remove unnecessary components --------- Co-authored-by: Jim Purvis <[email protected]> Co-authored-by: Iain Cooper <[email protected]> Co-authored-by: IlyasBaqqari-CabinetOffice <[email protected]> Co-authored-by: IlyasBaqqari-CabinetOffice <[email protected]> Co-authored-by: kiramarstonTCO <[email protected]> * fix copy (#359) * GAP-2138: Remove editor (#344) * Triggerring pipelines on PRs to feature branches * Triggerring pipelines on PRs to feature branches * GAP-2138: WIP remove page UI complete * Removing unneeded prop * GAP-2138: Unit tests start * GAP-2138: Finish unit tests * GAP-2432: add ui to display last updated by on a grant advert (#358) * initial * refactor * fix tets * refactor buildadvert * fix type * amend tests * fix test * amend copy * GAP-2137: Fix banner width * doesn't show buttons if no submissions to download (#364) * if application is not published send to external application (#360) * Handle last updated edge cases (#367) * initial * add tests * fix dates * mock timezone * mr feedback * use correct scheduled date * Fix case on am/pm string (#369) * fix case on am/pm string * fix tests * use 12 hour time (#370) * GAP-2433: View last updated by (Applications) (#365) * TMI2-689 - Admin satisfaction survey better fix (#371) * TMI2-689 - removed unnecessary styling from div * TMI2-689 - Fixed alignment issues on satisfaction survey * GAP:2433 Fix bad reassignment of email (#372) * GAP-2452: Add delete to multiple choice and multiple select options (#363) * Added delete button to mutliple choice/select questions (as seen when editing a question. * Added unit tests (exact same format as the ones for editing the questions) to cover the delete button rendering. * Fixed unit tests by adding correct post action along with options * Refactored callServiceMethod by extracting code out to functions, making it more readable * make label more sensible Co-authored-by: Conor Fayle <[email protected]> Signed-off-by: paul-lawlor-tco <[email protected]> * PR requested chagnes --------- Signed-off-by: paul-lawlor-tco <[email protected]> Co-authored-by: Conor Fayle <[email protected]> * update scheduled advert copy (#373) * TMI2-689 - adds missing css to div (#375) * checks if application is published (#376) * GAP 2443: Updates to admin dashboard (#366) * First pass at changes. Tests currently broken. * updates to admin dash * fixing tests * adding a - when updated by is null * PR feedback * further PR feedback * PR feedback III * remove 'open in new tab' option for links in rich text editor (#377) * remove new tab link option from rich text editor * remove link target option from rich text editor * checks application form status before editing (#379) * TMI2-715 - Added UploadFile component to admin question preview, added disabled functionality (#380) * TMI2-717: 2 options on multiple select application answers (#378) * TMI2-717 - Default to two options rather than one on questions * TMI2-717 - Typo fix --------- Co-authored-by: Jim Purvis <[email protected]> * GAP-2454/GAP-2457: multiple editors - edit section title (#374) * GAP-2454 / GAP-2457 | get multiple editors working for section title with no email * GAP-2454 | display email of user that last updated on error page * GAP-2454 | small refactor * GAP-2454 | readd lastPublished * GAP-2454 | fix test * GAP-2454 | add tests * GAP-2454 | add ui test for error page * GAP-2454 | add tests * GAP-2454 | rename revision back to version --------- Co-authored-by: conor <[email protected]> * GAP-2450: Don't allow an admin to be demoted if they have a scheme (#368) * Added copies for deleting user with grants * Add variable to determin if the admin owns any grants. * Pushing to change branch - current issue w/ function to check owner. It's based on session ID so always checks the Super admin schemes (not the admin being viewed). * passed query param that determines if a user owns a scheme. * Added unit tests - will fail while in draft due to checkbox behaviour being a WIP. * Conditionally render copy * Checkbox is disabled conditionally. However issues with roles occur when the user is an owner. Still a WIP. * modified checbox component to have disab;ed options built in. Handled case of admin role not being selected when the user OWNS a grant. * removed comments * Fix unit tests and add cases for 'Change' button URL when a SA views a users details. * change identifier for disabled element * implemented PR changes for Checkbox component and updated relevant tests/stories. * Made role ids a constant object to improve readability * TMI2-702: error assigning department to user (#386) * passes new user roles through query * fixed test * GAP-2460 | implement multiple editors error for deleting section (#383) * GAP-2460 | implement multiple editors error for deleting section * GAP-2460 | use query param for version instead of path * GAP-2460 | fix test --------- Co-authored-by: conor <[email protected]> * GAP-2458 | GAP-2459 | GAP-2456: Multiple Editors - edit question title, type, options, max words, eligibility (#385) * GAP-2458|GAP-2460|GAP-2456 | get versioning working for all edit question journeys * GAP-2458 | fix tests * GAP-2458 | fix tests * remove console log Signed-off-by: Conor Fayle <[email protected]> * remove log Signed-off-by: Conor Fayle <[email protected]> --------- Signed-off-by: Conor Fayle <[email protected]> Co-authored-by: conor <[email protected]> * Adds check for deleted user (#384) Co-authored-by: Dylan Wright <[email protected]> * GAP-2461: Multiple Editors - delete question (#387) * GAP-2461 | add version param to delete question call * GAP-2461 | implement feature and fix and add tests --------- Co-authored-by: conor <[email protected]> * TMI2-674 - Adds redirect to satisfaction survey on add new grant link. Adds logic to handle redirect from survey based on query params. (#389) * GAP-2462: Multiple Editors section reorder (#388) * GAP-2462 | section reorder api call uses version * GAP-2462 | implement section order versioning and fix tests --------- Co-authored-by: conor <[email protected]> * update node version (#390) * update node version, as there are vulnerabilities in the image used from docker * update image name so we will always get the latest version of alpine available for that node version * GAP-2463 | multiple editors reorder question (#391) Co-authored-by: conor <[email protected]> * fixing date format in submission overview (#394) * GAP-2500 | decrypt editor emails in server (#382) * initial * fix import * update test path * fix tests * fix import * use mock decrypt * add decrypt assertion * populate env.example * rename field * update test * handle deletedUser * fix typo * handle deleted user again * feedback * Update packages/admin/src/pages/scheme/components/SchemeApplications.test.tsx Co-authored-by: jgunnCO <[email protected]> Signed-off-by: john-tco <[email protected]> * use deletedUser field * use updated field * fix test * push deleted editor fix * rename var --------- Signed-off-by: john-tco <[email protected]> Co-authored-by: jgunnCO <[email protected]> * GAP-2455 | Handle editor adding question to a deleted section (#393) * Conditional error text on multiple editors error page * fix tests * Update packages/admin/src/pages/build-application/[applicationId]/error-multiple-editors.page.tsx Co-authored-by: jgunnCO <[email protected]> Signed-off-by: john-tco <[email protected]> * feedback --------- Signed-off-by: john-tco <[email protected]> Co-authored-by: jgunnCO <[email protected]> * changed dates to 12 hour clock (#396) * GAP-2436: download application preview ODT (#397) * GAP-2436 | download application preview ODT * GAP-2436 | use jest.mocked instead of as Jest.mock --------- Co-authored-by: conor <[email protected]> * amend names of shcheme and advert name fields (#398) * amend names of shcheme and advert name fields * fix tests * GAP-2502: Wrapping edit section questions tables final column better (#392) * fix advert name (#400) * fix advert name * fix test * GAP-2446: Add error page for multiple advert editors (#395) * TMI2-616: checks application form status before editing (#401) * doesn't allow non-custom sections of removed adverts to be edited * added missing props * fixed test * Tmi2 616/cannot edit unpublished advert (#402) * doesn't allow non-custom sections of removed adverts to be edited * added missing props * fixed test * don't allow admin to edit eligibility and essential sections * Added tests * updated test * GAP-2505: Remove references to 'scheme-list' (#408) * TMI2-616: reverted changes for editing unpublished application (#405) * reverted changes for editing unpublished application * doesn't allow essential and eligibility sections to be edited * TMI2-720: view submissions link when logged out (#404) * adds redirect url * fixed test * pr comments * TMI2-718: don't update role if department is undefined (#410) * don't update role if department is undefined * updated test * fixed submission download pattern for uuid (#412) * GAP-2517: put application name into the downloaded application form summary odt (#411) * GAP-2517 | put application name into the downloaded odt * GAP-2517 | formatting and linting --------- Co-authored-by: conor <[email protected]> * TMI2-728 - Application preview update (#399) Co-authored-by: Jim Purvis <[email protected]> Co-authored-by: iaincooper-tco <[email protected]> * Tmi2 720/submission download pattern (#413) * fixed submission download pattern for uuid * added logs * added hint text and removed space (#414) * GAP-2437 - Add preview application form ui (#407) * add preview application ui * extract applicationId * add preview age tests * fix dashboard page tests * add dashboardpage tests * amend test assertions * handle undefined contact email * update meta doc title * add banner * GAP-2479 - add section overview ui (#409) * add section overview ui * readd preview.module.scss * fix header spacing * rename var * Feature/gap 2480 (#406) * skeleton for local dev * push steps * Added preview component and back button functionality * Logic to handle next question. Still a WIP as I need to handle undefined case, create button, and write unit tests * Added fucntionality for next preview button and changed PreviewInoutSwitch to accept a new prop (to disable text questions) * Covered cases for ESSENTIAL sections which weren't covered in the preview input switch component. * Altered backHref variable to redirect to correct location - the preview application rather than the dashboard * Unit tests for new switch cases in preview-input-switch component * Remove unused imports * Added error case for an invalid/non-existent question * Added unit tests and refactored index page --------- Co-authored-by: john-tco <[email protected]> * Map v2 Application form section data (#415) * Map v2 Application form section data * add preview page tests * add helper tests * update assertions * use early return * Added fix for back button functionality and respective unit tests. (#416) * Added fix for back button functionality and respective unit tests. * refactors from PR * refactor parameters * add v2 flag to back button --------- Co-authored-by: john-tco <[email protected]> * GAP-2524 | change manage editors support email link to mailto (#419) Co-authored-by: conor <[email protected]> * Multiple editors - handle editing recently deleted section (#421) * handle section deleted by editor when fetching page data --------- Signed-off-by: john-tco <[email protected]> Signed-off-by: paul-lawlor-tco <[email protected]> Signed-off-by: Conor Fayle <[email protected]> Co-authored-by: john-tco <[email protected]> Co-authored-by: dominicwest <[email protected]> Co-authored-by: kiramarstonTCO <[email protected]> Co-authored-by: ryan-tco <[email protected]> Co-authored-by: jimpurvisTCO <[email protected]> Co-authored-by: Jim Purvis <[email protected]> Co-authored-by: Iain Cooper <[email protected]> Co-authored-by: IlyasBaqqari-CabinetOffice <[email protected]> Co-authored-by: IlyasBaqqari-CabinetOffice <[email protected]> Co-authored-by: rachelswart <[email protected]> Co-authored-by: paul-lawlor-tco <[email protected]> Co-authored-by: Conor Fayle <[email protected]> Co-authored-by: GavCookCO <[email protected]> Co-authored-by: jgunnCO <[email protected]> Co-authored-by: conor <[email protected]> Co-authored-by: Dylan Wright <[email protected]> Co-authored-by: a-lor-cab <[email protected]> Co-authored-by: iaincooper-tco <[email protected]> Co-authored-by: john-tco <[email protected]> Co-authored-by: JamieGunnCO <[email protected]>
Description
Ticket # and link
https://technologyprogramme.atlassian.net/jira/software/projects/GAP/boards/216?selectedIssue=GAP-2140
Adds ui for viewing and managing editors
Type of change
Please check the relevant options.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes:
Unit Test
Integration Test (if applicable)
End to End Test (if applicable)
Screenshots (if appropriate):
Please attach screenshots of the change if it is a UI change:
Checklist: