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

feat: [Duplicate Applet] Enhance Duplication (M2-7729) #1942

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from

Conversation

sultanofcardio
Copy link
Contributor

  • Tests for the changes have been added

📝 Description

🔗 Jira Ticket M2-7729

This PR adds a checkbox to the applet duplication popups for including report server configuration. The checkbox is optional and duplication works the same without checking it.

When the checkbox is checked, the duplicated applet will contain these properties from the original applet:

  • reportServerIp
  • reportPublicKey
  • reportIncludeUserId
  • reportIncludeCaseId
  • reportEmailBody

Note

reportRecipients is explicitly excluded

📸 Screenshots

Screen.Recording.2024-10-11.at.11.41.48.AM.mov

🪤 Peer Testing

  1. Create an applet with a report server configuration (you can use the dev server)
  2. Duplicate the applet both with and without the report server configuration (monitor dev tools network requests)
  3. Confirm that the applet can be duplication without checking the box
  4. Confirm that requests are sent to the report server /verify and /set-password endpoints
  5. Confirm that the applet duplicated with report server configuration has the properties mentioned above in the applet builder

✏️ Notes

N/A

@sultanofcardio sultanofcardio self-assigned this Oct 11, 2024
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-1942.d19gtpld8yi51u.amplifyapp.com

Copy link
Contributor

@mbanting mbanting left a comment

Choose a reason for hiding this comment

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

Great job @sultanofcardio!
Did some brief peer testing for duplicating with and without report server configuration, and it worked as expected. Left just found one small suggestion.

I also created M2-8037 to add unit tests later and added TODOs referencing this followup ticket. I didn't see that this task was already captured as part of M2-8036. So that one was updated to just focus on unit tests for the other ticket, M2-7738.

Pre-approving now, and moving to Ready for QA since the suggested change won't impact functional testing.

import { InputController } from 'shared/components/FormComponents';
import { StyledErrorText, StyledModalWrapper, variables } from 'shared/styles';
import { CheckboxController, InputController } from 'shared/components/FormComponents';
import { StyledErrorText, StyledFlexColumn, StyledModalWrapper, variables } from 'shared/styles';
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs StyledBodyLarge for use below

Suggested change
import { StyledErrorText, StyledFlexColumn, StyledModalWrapper, variables } from 'shared/styles';
import {
StyledBodyLarge,
StyledErrorText,
StyledFlexColumn,
StyledModalWrapper,
variables,
} from 'shared/styles';

<CheckboxController
name={'includeReportServer'}
control={control}
label={t('duplicateAppletReportServer')}
Copy link
Contributor

Choose a reason for hiding this comment

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

label prop is expecting an Element

Suggested change
label={t('duplicateAppletReportServer')}
label={<StyledBodyLarge>{t('duplicateAppletReportServer')}</StyledBodyLarge>}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants