From 4660ccfe497b5cab93b8ed014ba95a00c796bac7 Mon Sep 17 00:00:00 2001 From: Jeff Daley Date: Fri, 5 Apr 2024 15:51:46 -0400 Subject: [PATCH] Add support for Google Groups in the approver field (#661) * Prep for groups in the PeopleSelect * Explore XDropdownList * More basicDropdown discovery * Improve types and styles * Update nav.hbs * Add offset, improve styles * Improve dropdown design * Style tweak * WIP "Loading" * Improve loading states * Cleanup * Update people-select-test.ts * Tweak styles and offset * Prevent dropdown from opening on ArrowUp/Down * Set up `includeGroups` arg * Revert unnecessary change * Flatten array instead of groups * Reduce diff * Start of EmberData files * Clean Mirage People route * More prep for EmberData requests * Add notes from meeting with Josh * Rename/reorganize * Post-merge resolutions * Add conditional approve button and text * Improve group handling * Add group logic to maybeFetchPeople * Track cachedValue in `editableField` * Fix test; WIP update-editable-field function * Put TODO catch block on OPTIONS call * Get list to update on groupApproverClick * Remove approvers from drafts POST request - this isn't used in the frontend * Add groups API * Support group approvals * Forgot document group review model * WIP OPTIONS call * Add members of approver groups as individual approvers in the database * Load group with `maybeFetchPeople` * Filter departed users * Fix errors * Add/update tests; cleanup and documentation * Enable setting a Google Groups prefix * Only apply prefix if it looks like the user isn't beginning to type it * Add "remove me" assertion * Return results with and without a configured groups prefix --------- Co-authored-by: Josh Freda --- web/app/adapters/group.ts | 24 +++++ web/app/components/document/index.hbs | 1 + web/app/components/document/index.ts | 1 + web/app/components/document/sidebar.hbs | 89 +++++++++--------- web/app/components/document/sidebar.ts | 91 +++++++++++++++---- web/app/components/editable-field.hbs | 1 + web/app/components/editable-field.ts | 1 + web/app/components/inputs/people-select.hbs | 19 ++-- web/app/components/inputs/people-select.ts | 60 +++++++++--- web/app/components/person/avatar.hbs | 2 +- web/app/components/person/avatar.ts | 8 ++ web/app/components/person/index.hbs | 2 +- web/app/components/person/index.ts | 4 +- web/app/controllers/authenticated/document.ts | 5 + web/app/helpers/get-model-attr.ts | 5 +- web/app/models/group.ts | 13 +++ web/app/routes/authenticated/document.ts | 27 +++++- web/app/serializers/group.ts | 44 +++++++++ web/app/serializers/person.ts | 12 +-- web/app/services/_store.ts | 27 +++++- web/app/styles/app.scss | 1 + web/app/styles/components/multiselect.scss | 14 ++- web/app/styles/components/person.scss | 4 + web/app/templates/authenticated/document.hbs | 7 +- web/app/types/document.d.ts | 1 + web/app/utils/get-model-attr.ts | 21 ++++- web/mirage/config.ts | 50 ++++++++-- web/mirage/factories/group.ts | 6 ++ web/mirage/models/group.ts | 5 + .../acceptance/authenticated/document-test.ts | 68 ++++++++++++++ .../components/inputs/people-select-test.ts | 52 +++++++++-- .../components/person/avatar-test.ts | 27 +++++- .../helpers/get-model-attrs-test.ts | 63 ++++++++++++- 33 files changed, 632 insertions(+), 123 deletions(-) create mode 100644 web/app/adapters/group.ts create mode 100644 web/app/models/group.ts create mode 100644 web/app/serializers/group.ts create mode 100644 web/app/styles/components/person.scss create mode 100644 web/mirage/factories/group.ts create mode 100644 web/mirage/models/group.ts diff --git a/web/app/adapters/group.ts b/web/app/adapters/group.ts new file mode 100644 index 000000000..aa50d27d8 --- /dev/null +++ b/web/app/adapters/group.ts @@ -0,0 +1,24 @@ +import DS from "ember-data"; +import ApplicationAdapter from "./application"; +import RSVP from "rsvp"; + +export default class GroupAdapter extends ApplicationAdapter { + /** + * The Query method for the group model. + * Returns an array of groups that match the query. + * Also used by the `queryRecord` method. + */ + query(_store: DS.Store, _type: DS.Model, query: { query: string }) { + const results = this.fetchSvc + .fetch(`/api/${this.configSvc.config.api_version}/groups`, { + method: "POST", + body: JSON.stringify({ + // Spaces throw an error, so we replace them with dashes + query: query.query.replace(" ", "-"), + }), + }) + .then((r) => r?.json()); + + return RSVP.hash({ results }); + } +} diff --git a/web/app/components/document/index.hbs b/web/app/components/document/index.hbs index 03b3b4262..af5ba35c9 100644 --- a/web/app/components/document/index.hbs +++ b/web/app/components/document/index.hbs @@ -13,6 +13,7 @@ @docType={{@docType}} @isCollapsed={{this.sidebarIsCollapsed}} @toggleCollapsed={{this.toggleSidebarCollapsedState}} + @viewerIsGroupApprover={{@viewerIsGroupApprover}} /> {{/unless}} diff --git a/web/app/components/document/index.ts b/web/app/components/document/index.ts index 2012da65d..6cca0e784 100644 --- a/web/app/components/document/index.ts +++ b/web/app/components/document/index.ts @@ -11,6 +11,7 @@ interface DocumentIndexComponentSignature { document: HermesDocument; modelIsChanging: boolean; docType: Promise; + viewerIsGroupApprover: boolean; }; } diff --git a/web/app/components/document/sidebar.hbs b/web/app/components/document/sidebar.hbs index 5d4733f90..6103f6657 100644 --- a/web/app/components/document/sidebar.hbs +++ b/web/app/components/document/sidebar.hbs @@ -284,11 +284,12 @@ @@ -333,7 +334,7 @@ @model={{project.id}} class="related-resource-link quarternary-button peer flex h-[32px] items-center gap-2 px-[5px]" > -
+
@@ -475,9 +476,8 @@
{{else}} {{#let (and this.isDraft this.isOwner) as |canPublish|}} - {{#if (or canPublish this.isApprover)}} + {{#if (or canPublish this.isApprover this.isGroupApproverOnly)}}
- {{#if canPublish}} {{! Publish for review... }} {{else}} - {{! isApprover }} + {{! isApprover or isGroupApproverOnly}} {{! Read-only / isRunning state }} {{#if @@ -540,6 +540,7 @@ class="w-full" {{on "click" (perform this.approve)}} /> + {{#if (eq @document.docType "FRD")}} {{! Reject FRD }} {{/if}} - {{! Overflow menu (Leave approver role) }} - - <:anchor as |dd|> - - - - - <:item as |dd|> - - - {{dd.attrs.label}} - - - + + {{#unless this.isGroupApproverOnly}} + {{! Overflow menu (Leave approver role) }} + + <:anchor as |dd|> + + + + + <:item as |dd|> + + + {{dd.attrs.label}} + + + + {{/unless}} {{/if}} {{/if}}
@@ -763,6 +767,7 @@ ; isCollapsed: boolean; + viewerIsGroupApprover: boolean; toggleCollapsed: () => void; }; } @@ -71,6 +73,7 @@ export default class DocumentSidebarComponent extends Component e === this.args.profile.email, @@ -421,22 +452,6 @@ export default class DocumentSidebarComponent extends Component { + await this.patchDocument.perform({ + approvers: this.approvers, + approverGroups: this.approverGroups, + }); + }); + patchDocument = enqueueTask(async (fields: any, throwOnError?: boolean) => { const endpoint = this.isDraft ? "drafts" : "documents"; @@ -948,8 +978,20 @@ export default class DocumentSidebarComponent extends Component { + return this.store.peekRecord("group", approver); + }); + + this.approvers = approvers.filter((approver) => { + if (!this.approverGroups.includes(approver)) { + return this.store.peekRecord("person", approver); + } + }); } @action updateContributors(contributors: string[]) { @@ -1099,6 +1141,15 @@ export default class DocumentSidebarComponent extends Component
- +
-
- {{get-model-attr "person.name" value}} + + {{get-model-attr "person.name" option fallback="group.name"}} - {{get-model-attr "person.email" value}} + {{option}} -
+
diff --git a/web/app/components/inputs/people-select.ts b/web/app/components/inputs/people-select.ts index c39c51343..9b89a5f2e 100644 --- a/web/app/components/inputs/people-select.ts +++ b/web/app/components/inputs/people-select.ts @@ -11,6 +11,7 @@ import PersonModel from "hermes/models/person"; import { Select } from "ember-power-select/components/power-select"; import { next, schedule } from "@ember/runloop"; import calculatePosition from "ember-basic-dropdown/utils/calculate-position"; +import GroupModel from "hermes/models/group"; import AuthenticatedUserService from "hermes/services/authenticated-user"; export interface GoogleUser { @@ -46,6 +47,7 @@ interface InputsPeopleSelectComponentSignature { onChange: (value: string[]) => void; renderInPlace?: boolean; disabled?: boolean; + includeGroups?: boolean; onKeydown?: (dropdown: any, event: KeyboardEvent) => void; /** @@ -82,7 +84,7 @@ export default class InputsPeopleSelectComponent extends Component p.email) .filter((email: string) => { // filter out any people already selected @@ -231,7 +244,7 @@ export default class InputsPeopleSelectComponent extends Component selectedEmail === email, ); }) - .filter((email: string) => { + .filter((email) => { // filter the authenticated user if `excludeSelf` is true return ( !this.args.excludeSelf || @@ -239,9 +252,34 @@ export default class InputsPeopleSelectComponent extends Component { + const name = g.name.toLowerCase(); + if (name.includes("departed") || name.includes("terminated")) { + return false; + } else { + return true; + } + }) + .map((g: GroupModel) => g.email) + .filter((email) => { + // Filter out any people already selected + return !this.args.selected.find( + (selectedEmail) => selectedEmail === email, + ); + }); + } else { + g = []; } - // stop the loop if the query was successful + + // Concatenate and sort alphabetically + this.options = [...p, ...g].sort((a, b) => a.localeCompare(b)); + + // Stop the loop if the query was successful return; } catch (e) { // Throw an error if this is the last retry. diff --git a/web/app/components/person/avatar.hbs b/web/app/components/person/avatar.hbs index 0f52a85ea..43bbda2ba 100644 --- a/web/app/components/person/avatar.hbs +++ b/web/app/components/person/avatar.hbs @@ -27,7 +27,7 @@
{{/if}} diff --git a/web/app/components/person/avatar.ts b/web/app/components/person/avatar.ts index 6b3c95d47..c0a62f811 100644 --- a/web/app/components/person/avatar.ts +++ b/web/app/components/person/avatar.ts @@ -32,6 +32,14 @@ export default class PersonAvatarComponent extends Component +
{{#if this.badgeIsShown}} diff --git a/web/app/components/person/index.ts b/web/app/components/person/index.ts index da10ac061..6310dd895 100644 --- a/web/app/components/person/index.ts +++ b/web/app/components/person/index.ts @@ -27,7 +27,9 @@ export default class PersonComponent extends Component } return ( - getModelAttr(this.store, ["person.name", this.args.email]) ?? + getModelAttr(this.store, ["person.name", this.args.email], { + fallback: "group.name", + }) ?? this.args.email ?? "Unknown" ); diff --git a/web/app/controllers/authenticated/document.ts b/web/app/controllers/authenticated/document.ts index 8d6d161e4..e3add9d11 100644 --- a/web/app/controllers/authenticated/document.ts +++ b/web/app/controllers/authenticated/document.ts @@ -1,9 +1,14 @@ import Controller from "@ember/controller"; import { tracked } from "@glimmer/tracking"; +import AuthenticatedDocumentRoute from "hermes/routes/authenticated/document"; +import { ModelFrom } from "hermes/types/route-models"; export default class AuthenticatedDocumentController extends Controller { + declare model: ModelFrom; + queryParams = ["draft"]; draft = false; + /** * Whether the model is loading a new document from another one, * as is when loading a related Hermes document. diff --git a/web/app/helpers/get-model-attr.ts b/web/app/helpers/get-model-attr.ts index b6c795f11..aebb3cd95 100644 --- a/web/app/helpers/get-model-attr.ts +++ b/web/app/helpers/get-model-attr.ts @@ -6,6 +6,7 @@ import getModelAttr, { GetModelAttrArgs } from "hermes/utils/get-model-attr"; export interface GetModelAttrSignature { Args: { Positional: GetModelAttrArgs; + Named: { fallback?: string }; }; Return: any; } @@ -13,8 +14,8 @@ export interface GetModelAttrSignature { export default class GetModelAttrHelper extends Helper { @service declare store: StoreService; - compute(positional: GetModelAttrArgs) { - return getModelAttr(this.store, positional); + compute(positional: GetModelAttrArgs, named: { fallback?: string }) { + return getModelAttr(this.store, positional, named); } } diff --git a/web/app/models/group.ts b/web/app/models/group.ts new file mode 100644 index 000000000..5c63327b9 --- /dev/null +++ b/web/app/models/group.ts @@ -0,0 +1,13 @@ +import Model, { attr } from "@ember-data/model"; + +export default class GroupModel extends Model { + /** + * The name of the group, e.g., "Team Hermes" + */ + @attr declare name: string; + + /** + * The group's email address, e.g., "team-hermes@hashicorp.com" + */ + @attr declare email: string; +} diff --git a/web/app/routes/authenticated/document.ts b/web/app/routes/authenticated/document.ts index 987c68d47..1c81ecde9 100644 --- a/web/app/routes/authenticated/document.ts +++ b/web/app/routes/authenticated/document.ts @@ -23,6 +23,7 @@ interface AuthenticatedDocumentRouteParams { interface DocumentRouteModel { doc: HermesDocument; docType: HermesDocumentType; + viewerIsGroupApprover: boolean; } export enum DocStatus { @@ -152,20 +153,41 @@ export default class AuthenticatedDocumentRoute extends Route { } } + let viewerIsGroupApprover = false; + + // Check if the user is a group approver. + + const resp = await this.fetchSvc + .fetch( + `/api/${this.configSvc.config.api_version}/approvals/${params.document_id}`, + { method: "OPTIONS" }, + ) + .then((r) => r); + + const allowed = resp?.headers.get("allowed"); + + if (allowed?.includes("POST")) { + viewerIsGroupApprover = true; + } + const typedDoc = doc as HermesDocument; typedDoc.isDraft = typedDoc.status === "WIP"; + // Push the document's people into the store. + if (typedDoc.contributors?.length) { - // Add the contributors to the list of people to fetch. peopleToMaybeFetch.push(...typedDoc.contributors); } if (typedDoc.approvers?.length) { - // Add the approvers to the list of people to fetch. peopleToMaybeFetch.push(...typedDoc.approvers); } + if (typedDoc.approverGroups?.length) { + peopleToMaybeFetch.push(...typedDoc.approverGroups); + } + const customFields = typedDoc.customEditableFields; if (customFields) { @@ -206,6 +228,7 @@ export default class AuthenticatedDocumentRoute extends Route { return { doc: typedDoc, docType: this.docType(typedDoc), + viewerIsGroupApprover, }; } diff --git a/web/app/serializers/group.ts b/web/app/serializers/group.ts new file mode 100644 index 000000000..235e8201d --- /dev/null +++ b/web/app/serializers/group.ts @@ -0,0 +1,44 @@ +import JSONSerializer from "@ember-data/serializer/json"; +import { assert } from "@ember/debug"; +import DS from "ember-data"; +import GroupModel from "hermes/models/group"; + +interface GroupPayload { + results: GroupModel[] | null; +} + +export default class GroupSerializer extends JSONSerializer { + /** + * The serializer for the `group` model. + * Handles `query` requests to the EmberData store. + * Formats the response to match the JSON spec. + */ + normalizeResponse( + _store: DS.Store, + _primaryModelClass: GroupModel, + payload: GroupPayload, + _id: string | number | null, + _requestType: "query", + ) { + assert("results are expected for query requests", "results" in payload); + + /** + * If the results are `null`, return an empty array to show + * the "No results found" message in the PeopleSelect. + */ + if (!payload.results) return { data: [] }; + console.log(payload.results); + const groups = payload.results.map((g) => { + return { + id: g.email, + type: "group", + attributes: { + name: g.name, + email: g.email, + }, + }; + }); + + return { data: groups }; + } +} diff --git a/web/app/serializers/person.ts b/web/app/serializers/person.ts index 1bf72392e..942661126 100644 --- a/web/app/serializers/person.ts +++ b/web/app/serializers/person.ts @@ -27,15 +27,15 @@ export default class PersonSerializer extends JSONSerializer { */ if (!payload.results) return { data: [] }; - const people = payload.results.map((p: any) => { + const people = payload.results.map((p) => { return { - id: p.emailAddresses[0].value, + id: p.emailAddresses[0]?.value, type, attributes: { - name: p.names[0].displayName, - firstName: p.names[0].givenName, - email: p.emailAddresses[0].value, - picture: p.photos[0].url, + name: p.names[0]?.displayName, + firstName: p.names[0]?.givenName, + email: p.emailAddresses[0]?.value, + picture: p.photos[0]?.url, }, }; }); diff --git a/web/app/services/_store.ts b/web/app/services/_store.ts index 97a27b042..ac4fa6a6f 100644 --- a/web/app/services/_store.ts +++ b/web/app/services/_store.ts @@ -22,7 +22,7 @@ export default class StoreService extends Store { ) => { if (!emailsOrDocs) return; - let promises: Promise[] = []; + let promises: Promise[] = []; let uniqueEmails: string[] = []; emailsOrDocs = emailsOrDocs.uniq(); // Remove duplicates @@ -55,7 +55,8 @@ export default class StoreService extends Store { /** * Skip processing if the record is already in the store. */ - if (this.peekRecord("person", email)) return; + if (this.peekRecord("person", email) || this.peekRecord("group", email)) + return; /** * Skip emails already queued for processing. @@ -91,6 +92,28 @@ export default class StoreService extends Store { }); } }), + /** + * Groups API doesn't have a `findRecord` equivalent, so we query instead. + */ + this.query("group", { + query: email, + }).catch(() => { + /** + * Errors here are not necessarily indicative of a problem; + * for example, we get a 404 if a once-valid user is no longer in + * the directory. So we conditionally create a record for the email + * to prevent future requests for the same email. + */ + if (!email) return; + const cachedRecord = this.peekRecord("group", email); + + if (!cachedRecord) { + this.createRecord("group", { + id: email, + email, + }); + } + }), ); }); diff --git a/web/app/styles/app.scss b/web/app/styles/app.scss index d0e277cd3..8180fc50f 100644 --- a/web/app/styles/app.scss +++ b/web/app/styles/app.scss @@ -9,6 +9,7 @@ @use "./typography"; @use "components/dashboard"; +@use "components/person"; @use "components/new"; @use "components/toolbar"; @use "components/x-tooltip"; diff --git a/web/app/styles/components/multiselect.scss b/web/app/styles/components/multiselect.scss index 9b00a4c3c..4263aead8 100644 --- a/web/app/styles/components/multiselect.scss +++ b/web/app/styles/components/multiselect.scss @@ -45,7 +45,19 @@ } .ember-power-select-multiple-options { - @apply mr-6 flex h-full flex-wrap gap-2.5; + @apply mr-6 flex h-full w-full flex-wrap gap-2.5; + } + + .ember-power-select-option { + @apply py-px px-3.5; + + &:first-child { + @apply mt-[3px]; + } + + &:last-child { + @apply mb-[3px]; + } } .ember-power-select-trigger-multiple-input { diff --git a/web/app/styles/components/person.scss b/web/app/styles/components/person.scss new file mode 100644 index 000000000..b0e234175 --- /dev/null +++ b/web/app/styles/components/person.scss @@ -0,0 +1,4 @@ +.person { + @apply grid gap-2; + grid-template-columns: 20px 1fr; +} diff --git a/web/app/templates/authenticated/document.hbs b/web/app/templates/authenticated/document.hbs index 42333299e..a2615c30e 100644 --- a/web/app/templates/authenticated/document.hbs +++ b/web/app/templates/authenticated/document.hbs @@ -1,8 +1,9 @@ -{{page-title (or @model.doc.title "Document")}} +{{page-title (or this.model.doc.title "Document")}} {{set-body-class "secondary-screen"}} diff --git a/web/app/types/document.d.ts b/web/app/types/document.d.ts index 2fb78d9e1..44c8e39a9 100644 --- a/web/app/types/document.d.ts +++ b/web/app/types/document.d.ts @@ -32,6 +32,7 @@ export interface HermesDocument { appCreated?: boolean; contributors?: string[]; approvers?: string[]; + approverGroups?: string[]; changesRequestedBy?: string[]; approvedBy?: string[]; summary?: string; diff --git a/web/app/utils/get-model-attr.ts b/web/app/utils/get-model-attr.ts index dc8b31fcb..92ca22b7b 100644 --- a/web/app/utils/get-model-attr.ts +++ b/web/app/utils/get-model-attr.ts @@ -10,6 +10,9 @@ export type GetModelAttrArgs = [modelAndAttribute: string, id?: string]; export default function getModelAttr( store: StoreService, positional: GetModelAttrArgs, + named?: { + fallback?: string; + }, ) { const [modelAndAttribute, id] = positional; @@ -21,5 +24,21 @@ export default function getModelAttr( const record = store.peekRecord(model, id); - return record?.get(attribute); + if (!record) { + if (named?.fallback) { + const [model, attribute] = named.fallback.split("."); + + if (!(model && attribute)) return; + + const record = store.peekRecord(model, id); + + if (!record) return; + + return record.get(attribute); + } else { + return; + } + } else { + return record.get(attribute); + } } diff --git a/web/mirage/config.ts b/web/mirage/config.ts index 1eb9f4701..f37363ce9 100644 --- a/web/mirage/config.ts +++ b/web/mirage/config.ts @@ -7,6 +7,7 @@ import { ProjectStatus } from "hermes/types/project-status"; import { HITS_PER_PAGE } from "hermes/services/algolia"; import { PROJECT_HITS_PER_PAGE } from "hermes/routes/authenticated/projects/index"; import { assert as emberAssert } from "@ember/debug"; +import { HermesDocument } from "hermes/types/document"; import { FacetName } from "hermes/components/header/toolbar"; import { @@ -818,12 +819,20 @@ export default function (mirageConfig) { }); if (document) { - if (!document.attrs.approvedBy?.includes(TEST_USER_EMAIL)) { - const approvedBy = document.attrs.approvedBy || []; - document.update({ - approvedBy: [...approvedBy, TEST_USER_EMAIL], - }); - } + const { attrs } = document as { attrs: HermesDocument }; + const { approvedBy, approvers } = attrs; + + document.update({ + approvedBy: [...(approvedBy || []), TEST_USER_EMAIL], + /** + * Add any new approvers to the list, + * such as in the case of group approvals. + */ + approvers: approvers?.includes(TEST_USER_EMAIL) + ? approvers + : [...(approvers || []), TEST_USER_EMAIL], + }); + return new Response(200, {}, document.attrs); } @@ -849,6 +858,10 @@ export default function (mirageConfig) { return new Response(404, {}, {}); }); + this.options("/approvals/:document_id", (schema, request) => { + return new Response(200, {}, { allow: [] }); + }); + /************************************************************************* * * People @@ -857,13 +870,14 @@ export default function (mirageConfig) { // Query via the PeopleSelect this.post("/people", (schema, request) => { - let query: string = JSON.parse(request.requestBody).query; + let query: string = JSON.parse(request.requestBody).query.toLowerCase(); + // Search everyone's first emailAddress for matches let matches: Collection = schema["google/people"].where( (person) => { return ( - person.emailAddresses[0].value.includes(query) || - person.names[0].displayName.includes(query) + person.emailAddresses[0].value.toLowerCase().includes(query) || + person.names[0].displayName.toLowerCase().includes(query) ); }, ); @@ -872,6 +886,24 @@ export default function (mirageConfig) { return new Response(200, {}, matches.models); }); + /************************************************************************* + * + * Google Groups + * + ************************************************************************/ + + // Query via the PeopleSelect + this.post("/groups", (schema, request) => { + const requestBody = JSON.parse(request.requestBody); + const { query } = requestBody; + + const matches = schema.groups.where((group) => { + return group.email.includes(query) || group.name.includes(query); + }); + + return new Response(200, {}, matches.models); + }); + /************************************************************************* * * HEAD requests diff --git a/web/mirage/factories/group.ts b/web/mirage/factories/group.ts new file mode 100644 index 000000000..47acc8248 --- /dev/null +++ b/web/mirage/factories/group.ts @@ -0,0 +1,6 @@ +import { Factory } from "miragejs"; + +export default Factory.extend({ + name: (i: number) => `Group ${i}`, + email: (i: number) => `group-${i}@hashicorp.com`, +}); diff --git a/web/mirage/models/group.ts b/web/mirage/models/group.ts new file mode 100644 index 000000000..dea7fc045 --- /dev/null +++ b/web/mirage/models/group.ts @@ -0,0 +1,5 @@ +import { Model } from "miragejs"; + +export default Model.extend({ + // Required for Mirage, even though it's empty +}); diff --git a/web/tests/acceptance/authenticated/document-test.ts b/web/tests/acceptance/authenticated/document-test.ts index 36dc75697..6579cdae7 100644 --- a/web/tests/acceptance/authenticated/document-test.ts +++ b/web/tests/acceptance/authenticated/document-test.ts @@ -676,6 +676,41 @@ module("Acceptance | authenticated/document", function (hooks) { assert.true(changesRequestedBy?.includes(TEST_USER_EMAIL)); }); + test("group approvers can approve a document", async function (this: AuthenticatedDocumentRouteTestContext, assert) { + this.server.options("/approvals/:document_id", () => { + return new Response(200, { allowed: "POST" }, {}); + }); + + this.server.create("document", { + id: 1, + objectID: 1, + isDraft: false, + status: "In-review", + approvers: [], + approvedBy: [], + }); + + await visit("/document/1"); + + assert + .dom(OVERFLOW_MENU_BUTTON) + .doesNotExist( + 'the "remove me" function is not available to group approvers', + ); + + await click(APPROVE_BUTTON); + + assert.dom(SIDEBAR_FOOTER_PRIMARY_BUTTON_READ_ONLY).hasText("Approved"); + + assert.dom(APPROVERS_SELECTOR).containsText("Me"); + + const doc = this.server.schema.document.find(1).attrs; + + const { approvers } = doc; + + assert.true(approvers.includes(TEST_USER_EMAIL)); + }); + test("non-owner viewers of shareable drafts cannot edit the metadata of a draft", async function (this: AuthenticatedDocumentRouteTestContext, assert) { this.server.create("document", { objectID: 1, @@ -1739,4 +1774,37 @@ module("Acceptance | authenticated/document", function (hooks) { assert.dom(MODAL_ERROR).exists(); }); + + test("you can add a group as an approver", async function (this: AuthenticatedDocumentRouteTestContext, assert) { + const name = "Engineering"; + const email = "engineering@hashicorp.com"; + + this.server.create("group", { + name, + email, + }); + + this.server.create("document", { + objectID: 1, + id: 1, + }); + + await visit("/document/1?draft=true"); + + await click(`${APPROVERS_SELECTOR} button`); + + await fillIn(`${APPROVERS_SELECTOR} input`, name); + + assert.dom(PEOPLE_SELECT_OPTION).containsText(name).containsText(email); + + await click(PEOPLE_SELECT_OPTION); + + await click(EDITABLE_FIELD_SAVE_BUTTON_SELECTOR); + + assert.dom(APPROVERS_SELECTOR).containsText(name); + + const doc = this.server.schema.document.find(1).attrs; + + assert.true(doc.approverGroups.includes(email)); + }); }); diff --git a/web/tests/integration/components/inputs/people-select-test.ts b/web/tests/integration/components/inputs/people-select-test.ts index a15a4b1df..640b18424 100644 --- a/web/tests/integration/components/inputs/people-select-test.ts +++ b/web/tests/integration/components/inputs/people-select-test.ts @@ -1,10 +1,14 @@ import { module, test } from "qunit"; import { setupRenderingTest } from "ember-qunit"; import { hbs } from "ember-cli-htmlbars"; -import { click, fillIn, render, rerender, waitFor } from "@ember/test-helpers"; +import { click, fillIn, render, waitFor } from "@ember/test-helpers"; import { setupMirage } from "ember-cli-mirage/test-support"; import { MirageTestContext } from "ember-cli-mirage/test-support"; -import { TEST_USER_EMAIL, authenticateTestUser } from "hermes/mirage/utils"; +import { + TEST_USER_EMAIL, + authenticateTestUser, + pushMirageIntoStore, +} from "hermes/mirage/utils"; import { Response } from "miragejs"; const MULTISELECT = ".multiselect"; @@ -14,6 +18,9 @@ const OPTION = ".ember-power-select-option:not(.ember-power-select-option--no-matches-message)"; const NO_MATCHES_MESSAGE = ".ember-power-select-option--no-matches-message"; +const PERSON_COUNT = 10; +const GROUP_COUNT = 5; + interface PeopleSelectContext extends MirageTestContext { people: string[]; onChange: (newValue: string[]) => void; @@ -27,18 +34,31 @@ module("Integration | Component | inputs/people-select", function (hooks) { setupMirage(hooks); hooks.beforeEach(function (this: PeopleSelectContext) { - authenticateTestUser(this); - this.server.createList("google/person", 10); + this.server.createList("google/person", PERSON_COUNT); + this.server.createList("group", GROUP_COUNT); this.set("people", []); this.set("onChange", (newValue: string[]) => this.set("people", newValue)); + + authenticateTestUser(this); }); test("it functions as expected", async function (this: PeopleSelectContext, assert) { + this.server.create("group", { + name: "Departed User", + }); + + this.server.create("group", { + name: "User Who Was Terminated", + }); + + pushMirageIntoStore(this); + await render(hbs` `); @@ -50,25 +70,33 @@ module("Integration | Component | inputs/people-select", function (hooks) { assert .dom(OPTION) - .exists({ count: 10 }, "Options matching `u` are suggested"); + .exists( + { count: PERSON_COUNT + GROUP_COUNT }, + 'Options matching `u` are suggested, and groups containing "departed" or "terminated" are filtered out', + ); - await fillIn(INPUT, "1"); + await fillIn(INPUT, "user 1"); - assert.dom(OPTION).exists({ count: 2 }, "Results are filtered to match 1"); + assert + .dom(OPTION) + .exists( + { count: 2 }, + 'Results are filtered to match "user 1" (this will includes user 10)', + ); await click(OPTION); assert .dom(".ember-power-select-multiple-option .person-email") .hasText("User 1", "User 1 was successfully selected"); - await fillIn(INPUT, "2"); + await fillIn(INPUT, "User 2"); await click(OPTION); assert .dom(".ember-power-select-multiple-option .person-email") .exists({ count: 2 }, "User 2 was successfully selected"); - await fillIn(INPUT, "2"); + await fillIn(INPUT, "User 2"); assert .dom(NO_MATCHES_MESSAGE) .hasText("No results found", "No duplicate users can be added"); @@ -80,6 +108,12 @@ module("Integration | Component | inputs/people-select", function (hooks) { assert .dom(".ember-power-select-multiple-option .person-email") .exists({ count: 1 }, "People are removed from the list when clicked"); + + await fillIn(INPUT, "group"); + + assert + .dom(OPTION) + .exists({ count: GROUP_COUNT }, "Valid groups are suggested"); }); test("it will retry if the server returns an error", async function (this: PeopleSelectContext, assert) { diff --git a/web/tests/integration/components/person/avatar-test.ts b/web/tests/integration/components/person/avatar-test.ts index 55250f2bb..e02041a6b 100644 --- a/web/tests/integration/components/person/avatar-test.ts +++ b/web/tests/integration/components/person/avatar-test.ts @@ -1,11 +1,13 @@ -import { render } from "@ember/test-helpers"; +import { render, rerender } from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; import { setupRenderingTest } from "ember-qunit"; import { + TEST_USER_2_EMAIL, TEST_USER_EMAIL, TEST_USER_PHOTO, authenticateTestUser, + pushMirageIntoStore, } from "hermes/mirage/utils"; import { module, test } from "qunit"; @@ -95,4 +97,27 @@ module("Integration | Component | person/avatar", async function (hooks) { assert.dom(IMAGE).doesNotExist(); assert.dom(FALLBACK).exists(); }); + + test("people and groups have different fallback icons", async function (this: PersonAvatarTestContext, assert) { + this.server.create("group", { + id: TEST_USER_2_EMAIL, + email: TEST_USER_2_EMAIL, + }); + + pushMirageIntoStore(this); + + this.set("email", TEST_USER_2_EMAIL); + + await render(hbs` + + `); + + assert.dom(`${FALLBACK} svg`).hasAttribute("data-test-icon", "users"); + + this.set("email", "unknown"); + + await rerender(); + + assert.dom(`${FALLBACK} svg`).hasAttribute("data-test-icon", "user"); + }); }); diff --git a/web/tests/integration/helpers/get-model-attrs-test.ts b/web/tests/integration/helpers/get-model-attrs-test.ts index 1f5dcf091..a77a449f6 100644 --- a/web/tests/integration/helpers/get-model-attrs-test.ts +++ b/web/tests/integration/helpers/get-model-attrs-test.ts @@ -1,11 +1,13 @@ -import { module, test } from "qunit"; +import { log, module, test } from "qunit"; import { setupRenderingTest } from "ember-qunit"; -import { TestContext, find, render } from "@ember/test-helpers"; +import { TestContext, find, render, rerender } from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { MirageTestContext, setupMirage } from "ember-cli-mirage/test-support"; import { pushMirageIntoStore } from "hermes/mirage/utils"; -interface Context extends MirageTestContext {} +interface Context extends MirageTestContext { + email: string; +} module("Integration | Helper | get-model-attr", function (hooks) { setupRenderingTest(hooks); @@ -42,4 +44,59 @@ module("Integration | Helper | get-model-attr", function (hooks) { .dom(".non-model-attr") .hasText("Fallback", "it returns undefined for invalid attributes"); }); + + test("you can specify a fallback", async function (this: Context, assert) { + const groupName = "bar"; + const personName = "baz"; + const email = `${groupName}@hashicorp.com`; + + this.server.create("group", { + id: email, + name: groupName, + email, + }); + + pushMirageIntoStore(this); + + this.set("email", email); + + await render(hbs` +
+ {{get-model-attr "person.name" this.email fallback="group.name"}} +
+ `); + + assert + .dom(".name") + .hasText( + groupName, + "it didn't find a person match but it found a group match", + ); + + this.server.create("person", { + id: email, + name: personName, + email, + imgURL: undefined, + }); + + this.server.create("google/person", { + id: email, + }); + + pushMirageIntoStore(this); + + /** + * The `rerender` method doesn't work with this helper + * so we `render` the template instead. + */ + + await render(hbs` +
+ {{get-model-attr "person.name" this.email fallback="group.name"}} +
+ `); + + assert.dom(".name").hasText(personName, "it found a person match"); + }); });