fix(PayInvoiceModal): fix incorrect sats value displayed for paid inv…

[AdamuAbba]

Closes #1107

Description

This PR fixes a bug where an incorrect sats value is displayed by the notification for paid invoices (PayInvoiceModal)

Steps to Test

1. Ensure you have the most current version of the master branch.
2. Set up a network with multiple Lightning nodes.
3. Generate an invoice with one node and pay it using another.
4. Pay close attention to the success notification.

Screenshots

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ceaf84f) to head (084ab87).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #1110   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          154       154           
  Lines         5567      5567           
  Branches      1082      1128   +46     
=========================================
  Hits          5567      5567           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kelvinator07]

Hi @AdamuAbba, I suggest adding some tests to validate the fix.

[AmosOO7]

Hi @AdamuAbba, I ran the changes you made on my end and it did solve the value issue on the notification.

[AdamuAbba]

Hi @AdamuAbba, I suggest adding some tests to validate the fix.

Alright @kelvinator07 I'll get on it. Thank you.

[jamaljsr]

@AdamuAbba Thanks for this fix. LGTM 👌

During my testing of this PR, I noticed that there's also a similar issue with the amount in the notification when sending asset payments. Would you mind applying the fix for this here as well?

You'll just need to change this one line in tapdService.ts to

amount: parseInt(pmt.valueMsat) / 1000,

And also, update the unit test in tapdService.spec.ts to

valueMsat: 1_000_000,

[AdamuAbba]

Hey @kelvinator07, it's been a while. For the test, would you advise me to tweak the existing test (show belown) or duplicate it into a new unit test? Also, @jamaljsr, I've pushed the requested changes in my last commit, so once this test is tied in, the PR should be ready for review. sorry for the delay.

-  it('should pay invoice successfully', async () => {
+  it('should pay invoice successfully and show correctly formatted success message', async () => {
- 	  const { getByText, getByLabelText, store } = await renderComponent();
+ 	  const { getByText, getByLabelText, store, findByText } = await renderComponent();
      fireEvent.change(getByLabelText('BOLT 11 Invoice'), { target: { value: 'lnbc1' } });
      fireEvent.click(getByText('Pay Invoice'));
      await waitFor(() => {
        expect(store.getState().modals.payInvoice.visible).toBe(false);
      });
      const node = network.nodes.lightning[0];
      expect(lightningServiceMock.payInvoice).toHaveBeenCalledWith(
        node,
        'lnbc1',
        undefined,
      );
+ 	  const element = await findByText('Sent 1,000 sats from alice');
+	  expect(element).toBeInTheDocument();
    });

--- OR ---

  it('should display correctly formatted amount in paid invoice success notification', async () => {
      const { getByText, findByText, getByLabelText, store } = await renderComponent();
      fireEvent.change(getByLabelText('BOLT 11 Invoice'), { target: { value: 'lnbc1' } });
      fireEvent.click(getByText('Pay Invoice'));
      await waitFor(() => {
        expect(store.getState().modals.payInvoice.visible).toBe(false);
      });
      const node = network.nodes.lightning[0];
      expect(lightningServiceMock.payInvoice).toHaveBeenCalledWith(
        node,
        'lnbc1',
        undefined,
      );
      const element = await findByText('Sent 1,000 sats from alice');
      expect(element).toBeInTheDocument();
    });

[kelvinator07]

Hey @kelvinator07, it's been a while. For the test, would you advise me to tweak the existing test (show belown) or duplicate it into a new unit test? Also, @jamaljsr, I've pushed the requested changes in my last commit, so once this test is tied in, the PR should be ready for review. sorry for the delay.

-  it('should pay invoice successfully', async () => {
+  it('should pay invoice successfully and show correctly formatted success message', async () => {
- 	  const { getByText, getByLabelText, store } = await renderComponent();
+ 	  const { getByText, getByLabelText, store, findByText } = await renderComponent();
      fireEvent.change(getByLabelText('BOLT 11 Invoice'), { target: { value: 'lnbc1' } });
      fireEvent.click(getByText('Pay Invoice'));
      await waitFor(() => {
        expect(store.getState().modals.payInvoice.visible).toBe(false);
      });
      const node = network.nodes.lightning[0];
      expect(lightningServiceMock.payInvoice).toHaveBeenCalledWith(
        node,
        'lnbc1',
        undefined,
      );
+ 	  const element = await findByText('Sent 1,000 sats from alice');
+	  expect(element).toBeInTheDocument();
    });

--- OR ---

  it('should display correctly formatted amount in paid invoice success notification', async () => {
      const { getByText, findByText, getByLabelText, store } = await renderComponent();
      fireEvent.change(getByLabelText('BOLT 11 Invoice'), { target: { value: 'lnbc1' } });
      fireEvent.click(getByText('Pay Invoice'));
      await waitFor(() => {
        expect(store.getState().modals.payInvoice.visible).toBe(false);
      });
      const node = network.nodes.lightning[0];
      expect(lightningServiceMock.payInvoice).toHaveBeenCalledWith(
        node,
        'lnbc1',
        undefined,
      );
      const element = await findByText('Sent 1,000 sats from alice');
      expect(element).toBeInTheDocument();
    });

The first one is better. It will help reduce duplicate code and robustly test the pay invoice.

[AdamuAbba]

Hey @jamaljsr, I think this PR is ready for review when you have the time

[kelvinator07]

tACK. LGTM!