SV UI Look&Feel: Voting (#3445)

Signed-off-by: Paweł Perek <pawel.perek@digitalasset.com>

Author: pawelperek-da

apps/app/src/test/scala/org/lfdecentralizedtrust/splice/integration/tests/SvFrontendIntegrationTest.scala

@@ -454,18 +454,20 @@ class SvFrontendIntegrationTest
       )(
         "sv2 can see the vote request and cast a vote",
         _ => {
-          inside(find(id("your-vote-reason-input"))) { case Some(element) =>
+          eventuallySucceeds() {
+            find(testId("your-vote-reason-input")) should not be empty
+          }
+
+          inside(find(testId("your-vote-reason-input"))) { case Some(element) =>
             element.underlying.sendKeys("A sample reason")
           }
 
-          inside(find(id("your-vote-url-input"))) { case Some(element) =>
+          inside(find(testId("your-vote-url-input"))) { case Some(element) =>
             element.underlying.sendKeys("https://my-splice-vote-url.com")
           }
 
           click on testId("your-vote-accept")
 
-          eventuallyClickOn(id("submit-vote-button"))
-
           clue("wait for the vote submission success message") {
             eventuallySucceeds() {
               inside(find(testId("vote-submission-success"))) { case Some(element) =>
@@ -516,7 +518,7 @@ class SvFrontendIntegrationTest
     )(implicit
         env: SpliceTestConsoleEnvironment
     ): String = clue(s"Creating proposal: $action") {
-      val requestReasonUrl = "https://new-proposal-url.com/"
+      val requestReasonUrl = "new-proposal-url.com/"
       val requestReasonBody = "This is a summary of the proposal"
 
       val proposalContractId = withFrontEnd("sv1") { implicit webDriver =>

apps/sv/frontend/src/__tests__/governance/forms/create-unallocated-unclaimed-activity-form.test.tsx

@@ -65,7 +65,7 @@ describe('Create Unallocated Unclaimed Activity Record Form', () => {
 
     const urlInput = screen.getByTestId('create-unallocated-unclaimed-activity-record-url');
     expect(urlInput).toBeInTheDocument();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const beneficiaryInput = screen.getByTestId(
       'create-unallocated-unclaimed-activity-record-beneficiary'

apps/sv/frontend/src/__tests__/governance/forms/grant-revoke-featured-app-form.test.tsx

@@ -61,7 +61,7 @@ describe('Grant Featured App Form', () => {
 
     const urlInput = screen.getByTestId('grant-featured-app-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const idInput = screen.getByTestId('grant-featured-app-idValue');
     expect(idInput).toBeDefined();
@@ -231,7 +231,7 @@ describe('Revoke Featured App Form', () => {
 
     const urlInput = screen.getByTestId('revoke-featured-app-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const idInput = screen.getByTestId('revoke-featured-app-idValue');
     expect(idInput).toBeDefined();

apps/sv/frontend/src/__tests__/governance/forms/offboard-sv-form.test.tsx

@@ -61,7 +61,7 @@ describe('Offboard SV Form', () => {
 
     const urlInput = screen.getByTestId('offboard-sv-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const memberInput = screen.getByTestId('offboard-sv-member-dropdown');
     expect(memberInput).toBeDefined();

apps/sv/frontend/src/__tests__/governance/forms/set-amulet-rules-form.test.tsx

@@ -61,7 +61,7 @@ describe('Set Amulet Config Rules Form', () => {
 
     const urlInput = screen.getByTestId('set-amulet-config-rules-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     // Amulet Rules has a lot of fields to process so this can get flakey if not given enough time
     waitFor(

apps/sv/frontend/src/__tests__/governance/forms/set-dso-rules-form.test.tsx

@@ -59,7 +59,7 @@ describe('Set DSO Config Rules Form', () => {
 
     const urlInput = screen.getByTestId('set-dso-config-rules-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const configLabels = screen.getAllByTestId(/config-label-/);
     expect(configLabels.length).toBeGreaterThan(15);

apps/sv/frontend/src/__tests__/governance/forms/update-sv-reward-weight-form-test.test.tsx

@@ -61,7 +61,7 @@ describe('Update SV Reward Weight Form', () => {
 
     const urlInput = screen.getByTestId('update-sv-reward-weight-url');
     expect(urlInput).toBeDefined();
-    expect(urlInput.getAttribute('value')).toBe('');
+    expect(urlInput.getAttribute('value')).toBe('https://');
 
     const memberInput = screen.getByTestId('update-sv-reward-weight-member-dropdown');
     expect(memberInput).toBeDefined();

apps/sv/frontend/src/__tests__/governance/proposal-details-content.test.tsx

@@ -130,14 +130,30 @@ describe('SV user can', () => {
 
 describe('Proposal Details Content', () => {
   test('should render proposal details page', async () => {
+    const votesWithNoCurrentSvVote = [
+      {
+        sv: 'sv1',
+        isYou: true,
+        vote: 'no-vote' as const,
+      },
+      {
+        sv: 'sv3',
+        vote: 'rejected' as const,
+        reason: {
+          url: 'https://example.com',
+          body: 'Reason',
+        },
+      },
+    ];
+
     render(
       <Wrapper>
         <ProposalDetailsContent
           currentSvPartyId={voteRequest.votingInformation.requester}
           contractId={voteRequest.contractId}
           proposalDetails={voteRequest.proposalDetails}
           votingInformation={voteRequest.votingInformation}
-          votes={voteRequest.votes}
+          votes={votesWithNoCurrentSvVote}
         />
       </Wrapper>
     );
@@ -160,7 +176,7 @@ describe('Proposal Details Content', () => {
     const summary = screen.getByTestId('proposal-details-summary-value');
     expect(summary.textContent).toMatch(/Summary of the proposal/);
 
-    const url = screen.getByTestId('proposal-details-url-value');
+    const url = screen.getByTestId('proposal-details-url');
     expect(url.textContent).toMatch(/https:\/\/example.com/);
 
     const votingInformationSection = screen.getByTestId('proposal-details-voting-information');
@@ -263,7 +279,7 @@ describe('Proposal Details Content', () => {
     expect(unfeaturedAppSection).toBeInTheDocument();
 
     const rightContractId = screen.getByTestId('proposal-details-unfeature-app-label');
-    expect(rightContractId.textContent).toMatch(/Contract ID/);
+    expect(rightContractId.textContent).toMatch(/Proposal ID/);
 
     const rightContractIdValue = screen.getByTestId('proposal-details-unfeature-app-value');
     expect(rightContractIdValue.textContent).toMatch(/rightContractId/);
@@ -330,10 +346,8 @@ describe('Proposal Details Content', () => {
     const action = screen.getByTestId('proposal-details-action-value');
     expect(action.textContent).toMatch(/Create Unclaimed Activity Record/);
 
-    const beneficiary = screen
-      .getByTestId('proposal-details-beneficiary-input')
-      .getAttribute('value');
-    expect(beneficiary).toMatch(/sv1/);
+    const beneficiary = screen.getByTestId('proposal-details-beneficiary');
+    expect(beneficiary.textContent).toMatch(/sv1/);
 
     const amount = screen.getByTestId('proposal-details-amount-value');
     expect(amount.textContent).toMatch(/10/);
@@ -760,14 +774,18 @@ describe('Proposal Details > Votes & Voting', () => {
   });
 
   test('should render voting form for vote request when voting has not closed', () => {
+    const votesWithNoCurrentSvVote: ProposalVote[] = votesData.map(v =>
+      v.sv === 'sv1' ? { sv: v.sv, isYou: v.isYou, vote: 'no-vote' as const } : v
+    );
+
     render(
       <Wrapper>
         <ProposalDetailsContent
           currentSvPartyId={voteRequest.votingInformation.requester}
           contractId={voteRequest.contractId}
           proposalDetails={voteRequest.proposalDetails}
           votingInformation={voteRequest.votingInformation}
-          votes={votesData}
+          votes={votesWithNoCurrentSvVote}
         />
       </Wrapper>
     );
@@ -804,7 +822,7 @@ describe('Proposal Details > Votes & Voting', () => {
     expect(screen.queryByTestId('your-vote-form')).not.toBeInTheDocument();
   });
 
-  test('submit button says Submit if sv has not voted', async () => {
+  test('renders accept and reject buttons with correct labels', async () => {
     const votes: ProposalVote[] = [
       {
         sv: 'sv1',
@@ -823,39 +841,13 @@ describe('Proposal Details > Votes & Voting', () => {
     );
 
     const votingForm = screen.getByTestId('your-vote-form');
-    const submitButton = within(votingForm).getByTestId('submit-vote-button');
+    const acceptButton = within(votingForm).getByTestId('your-vote-accept');
+    const rejectButton = within(votingForm).getByTestId('your-vote-reject');
 
-    expect(submitButton).toBeInTheDocument();
-    expect(submitButton.textContent).toMatch(/Submit/);
-  });
-
-  test('submit button says Update if sv has already voted', async () => {
-    const votes: ProposalVote[] = [
-      {
-        sv: 'sv1',
-        vote: 'accepted',
-        reason: {
-          url: 'https://sv1.example.com',
-          body: 'SV1 Reason',
-        },
-      },
-    ];
-
-    render(
-      <Wrapper>
-        <ProposalVoteForm
-          voteRequestContractId={voteRequest.contractId}
-          currentSvPartyId={'sv1'}
-          votes={votes}
-        />
-      </Wrapper>
-    );
-
-    const votingForm = screen.getByTestId('your-vote-form');
-    const submitButton = within(votingForm).getByTestId('submit-vote-button');
-
-    expect(submitButton).toBeInTheDocument();
-    expect(submitButton.textContent).toMatch(/Update/);
+    expect(acceptButton).toBeInTheDocument();
+    expect(acceptButton.textContent).toMatch(/Accept/);
+    expect(rejectButton).toBeInTheDocument();
+    expect(rejectButton.textContent).toMatch(/Reject/);
   });
 
   test('render success message after api returns success', async () => {
@@ -901,20 +893,16 @@ describe('Proposal Details > Votes & Voting', () => {
     const reasonInput = within(votingForm).getByTestId('your-vote-reason-input');
     expect(reasonInput).toBeInTheDocument();
 
-    const acceptRadio = within(votingForm).getByTestId('your-vote-accept');
-    expect(acceptRadio).toBeInTheDocument();
-
-    await user.click(acceptRadio);
-
-    const submitButton = within(votingForm).getByTestId('submit-vote-button');
-    expect(submitButton).toBeInTheDocument();
+    const acceptButton = within(votingForm).getByTestId('your-vote-accept');
+    expect(acceptButton).toBeInTheDocument();
 
+    // Clicking the Accept button both selects the vote and submits
     // It's usually a good idea to await this click action. However this happens to be one where we shouldn't
     // This is because awaiting the button click makes it very difficult for the test runner to see the loading state
-    user.click(submitButton);
+    user.click(acceptButton);
 
     await waitFor(async () => {
-      expect(submitButton.getAttribute('disabled')).toBeDefined();
+      expect(acceptButton.getAttribute('disabled')).toBeDefined();
     });
 
     const submissionMessage = await screen.findByTestId('submission-message');
@@ -968,20 +956,16 @@ describe('Proposal Details > Votes & Voting', () => {
     const reasonInput = within(votingForm).getByTestId('your-vote-reason-input');
     expect(reasonInput).toBeInTheDocument();
 
-    const acceptRadio = within(votingForm).getByTestId('your-vote-accept');
-    expect(acceptRadio).toBeInTheDocument();
-
-    await user.click(acceptRadio);
-
-    const submitButton = within(votingForm).getByTestId('submit-vote-button');
-    expect(submitButton).toBeInTheDocument();
+    const acceptButton = within(votingForm).getByTestId('your-vote-accept');
+    expect(acceptButton).toBeInTheDocument();
 
+    // Clicking the Accept button both selects the vote and submits
     // It's usually a good idea to await this click action. However this happens to be one where we shouldn't
     // This is because awaiting the button click makes it very difficult for the test runner to see the loading state
-    user.click(submitButton);
+    user.click(acceptButton);
 
     await waitFor(async () => {
-      expect(submitButton.getAttribute('disabled')).toBeDefined();
+      expect(acceptButton.getAttribute('disabled')).toBeDefined();
     });
 
     const submissionMessage = await screen.findByTestId('submission-message');
@@ -1028,17 +1012,17 @@ describe('Proposal Details > Votes & Voting', () => {
 
     await user.type(urlInput, 'invalid_url');
 
-    const acceptRadio = within(votingForm).getByTestId('your-vote-accept');
-    user.click(acceptRadio);
+    const acceptButton = within(votingForm).getByTestId('your-vote-accept');
+    const rejectButton = within(votingForm).getByTestId('your-vote-reject');
 
-    const submitButton = screen.getByTestId('submit-vote-button');
-    expect(submitButton.getAttribute('disabled')).toBeDefined();
+    expect(acceptButton).toBeDisabled();
+    expect(rejectButton).toBeDisabled();
 
     const urlHelperText = within(votingForm).getByTestId('your-vote-url-helper-text');
     expect(urlHelperText.textContent).toMatch(/Invalid URL/);
   });
 
-  test('prevent submission if vote has not been chosen', async () => {
+  test('renders accept and reject buttons', async () => {
     const votes: ProposalVote[] = [
       {
         sv: 'sv1',
@@ -1054,8 +1038,6 @@ describe('Proposal Details > Votes & Voting', () => {
       },
     ];
 
-    const user = userEvent.setup();
-
     render(
       <Wrapper>
         <ProposalVoteForm
@@ -1069,12 +1051,12 @@ describe('Proposal Details > Votes & Voting', () => {
     const votingForm = screen.getByTestId('your-vote-form');
     expect(votingForm).toBeInTheDocument();
 
-    const submitButton = screen.getByTestId('submit-vote-button');
-    expect(submitButton.getAttribute('disabled')?.valueOf()).toBe('');
-
-    const rejectRadio = within(votingForm).getByTestId('your-vote-reject');
-    await user.click(rejectRadio);
+    const acceptButton = within(votingForm).getByTestId('your-vote-accept');
+    const rejectButton = within(votingForm).getByTestId('your-vote-reject');
 
-    expect(submitButton.getAttribute('disabled')?.valueOf()).toBe(undefined);
+    expect(acceptButton).toBeInTheDocument();
+    expect(rejectButton).toBeInTheDocument();
+    expect(acceptButton).not.toBeDisabled();
+    expect(rejectButton).not.toBeDisabled();
   });
 });

apps/sv/frontend/src/components/beta/CopyableIdentifier.tsx

@@ -7,13 +7,15 @@ export type CopyableIdentifierSize = 'small' | 'large';
 
 interface CopyableIdentifierProps {
   value: string;
+  copyValue?: string;
   badge?: string;
   size: CopyableIdentifierSize;
   'data-testid': string;
 }
 
 const CopyableIdentifier: React.FC<CopyableIdentifierProps> = ({
   value,
+  copyValue,
   badge,
   size,
   'data-testid': testId,
@@ -32,7 +34,7 @@ const CopyableIdentifier: React.FC<CopyableIdentifierProps> = ({
     <IconButton
       color="secondary"
       data-testid={`${testId}-copy-button`}
-      onClick={() => navigator.clipboard.writeText(value)}
+      onClick={() => navigator.clipboard.writeText(copyValue ?? value)}
     >
       <ContentCopy sx={{ fontSize: size === 'small' ? '14px' : '18px' }} />
     </IconButton>