Skip to content
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

[MPDX-8193] Preselect new appeal options #1059

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Conversation

canac
Copy link
Contributor

@canac canac commented Sep 11, 2024

Description

  • Initially set Add contacts with the following status(es): to -- All Active --
  • Initially set Do not add contacts who: to Have "Send Appeals" set to No

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@canac canac added the Preview Environment Add this label to create an Amplify Preview label Sep 11, 2024
@canac canac requested a review from dr-bizz September 11, 2024 21:38
Copy link
Contributor

Copy link
Contributor

github-actions bot commented Sep 11, 2024

Bundle sizes [mpdx-react]

Compared against 0712f37

No significant changes found

Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Can we move the logic inside the AddAppealForm? As these settings are standard wherever the form will be used.

Comment on lines 72 to 84
<AddAppealForm
accountListId={accountListId || ''}
appealStatuses={[
{
name: '-- All Active --',
value: 'ACTIVE',
},
]}
appealExcludes={contactExclusions.filter(
(exclusion) =>
exclusion.value === ExclusionEnum.DoNotAskAppeals,
)}
/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes need to be added to the form itself as this is the default.

Can you move these changes inside the <AddAppealForm/>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does AddAppealForm accept props to initialize the form then? Can those props all be removed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It accepts props as they will be used in the announcements, which can pass down appeal statements and appeal excludes.

Can we make the appealStatuses and appealExcludes have a default value of what you have above?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It uses it in the old one here: src/tools/appeals/appeals.service.ts Line 62 "oneClickAppeal"

@canac canac requested a review from dr-bizz September 13, 2024 16:47
@canac canac force-pushed the 8193-appeal-preselected-options branch from 5a525e6 to ada2107 Compare September 13, 2024 17:29
Copy link
Contributor

@dr-bizz dr-bizz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!

@canac canac merged commit ea0b4ee into main Sep 13, 2024
18 checks passed
@canac canac deleted the 8193-appeal-preselected-options branch September 13, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants