DEVPROD-948: Allow setting Parsley filters for repos #2135
Conversation
Passing run #13729 ↗︎
Details:
Review all test suite changes for PR #2135 ↗︎ |
||||||||||||||||||||
| @@ -2,44 +2,15 @@ import { GetFormSchema } from "components/SpruceForm"; | |||
| import { CardFieldTemplate } from "components/SpruceForm/FieldTemplates"; | |||
| import widgets from "components/SpruceForm/Widgets"; | |||
| import { ProjectHealthView } from "gql/generated/types"; | |||
| import { ProjectType } from "../utils"; | |||
|
|
|||
There was a problem hiding this comment.
Should I be able to see the repo filters on Parsley in the project filters modal? I'm testing using localhost, but I don't see them... :o
There was a problem hiding this comment.
Thank you, I opened an Evergreen PR to address this: evergreen-ci/evergreen#7200
| @@ -2,44 +2,15 @@ import { GetFormSchema } from "components/SpruceForm"; | |||
| import { CardFieldTemplate } from "components/SpruceForm/FieldTemplates"; | |||
| import widgets from "components/SpruceForm/Widgets"; | |||
| import { ProjectHealthView } from "gql/generated/types"; | |||
| import { ProjectType } from "../utils"; | |||
|
|
|||
There was a problem hiding this comment.
What happens if the repo and project filter use the same filter expression? Should they count as separate filters or should they be deduplicated? (For the project filters, it errors if you try to define the same filter expression more than once)
There was a problem hiding this comment.
Good callout, added repo values to the same validation
| ...(projectType !== ProjectType.Repo && | ||
| "projectHealthView" in projectRef && { |
There was a problem hiding this comment.
Is it necessary to check the ProjectType?
| ...(projectType !== ProjectType.Repo && | |
| "projectHealthView" in projectRef && { | |
| ...("projectHealthView" in projectRef && { |
There was a problem hiding this comment.
Not strictly necessary, but this is the pattern we've used on other pages for non-repo settings and I feel like it makes it clear that that's the intended purpose.
| ...("view" in formState && { | ||
| projectHealthView: formState.view.projectHealthView, | ||
| }), |
There was a problem hiding this comment.
Is it possible to keep the destructuring on L31 and then do:
| ...("view" in formState && { | |
| projectHealthView: formState.view.projectHealthView, | |
| }), | |
| ...(view && { | |
| projectHealthView: view.projectHealthView, | |
| }), |
| @@ -1,13 +1,16 @@ | |||
| import { ProjectHealthView, ProjectSettingsInput } from "gql/generated/types"; | |||
| import { data } from "../testData"; | |||
| import { ProjectType } from "../utils"; | |||
| import { formToGql, gqlToForm } from "./transformers"; | |||
| import { ViewsFormState } from "./types"; | |||
|
|
|||
| const { projectBase } = data; | |||
|
|
|||
| describe("project data", () => { | |||
There was a problem hiding this comment.
i think another test using the repo data showing that the project health view gets correctly omitted would be nice!
DEVPROD-948
Description
Fun update: because thenvm 😢 small backend update required: DEVPROD-948: Return repo level filters with project evergreen#7200Projectresolver applies theincludeRepoflag, Parsley already handles applying repo-level filters without any further updates 🎉 you can try it out as part of reviewing this PR! I personally don't think there's any need to distinguish a project vs repo-level filter on the Parsley UI.Screenshots
Repo view:
Project view:
