Skip to content

Add invoice module E2E tests for payment management#95

Open
vaneck237 wants to merge 5 commits intoopenimis:developfrom
Y-Note-SAS:feature-35923-tests
Open

Add invoice module E2E tests for payment management#95
vaneck237 wants to merge 5 commits intoopenimis:developfrom
Y-Note-SAS:feature-35923-tests

Conversation

@vaneck237
Copy link
Copy Markdown

@vaneck237 vaneck237 commented Mar 24, 2026

Description

This is an implementation of E2E test for add and delete invoice payment

The testing process is as follows:

  • Create a family
  • Assign a policy
  • Search for the invoice using the head of household’s insured number
  • Create a payment for the invoice
  • Delete the payment

Type of Change

  • Feature
  • Bug fix
  • Chore (Refactor, Docs, CI/CD)
  • Other, please specify

Related Issue(s) / Task(s)

Demo

Screen.Recording.2026-04-21.at.2.06.01.PM.mov

Checklist

  • Unit tests added/modified
  • I18n / translation handled

Copy link
Copy Markdown
Contributor

@Shahzaibahmad97 Shahzaibahmad97 left a comment

Choose a reason for hiding this comment

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

I have added my review, there are some code refactoring suggestions along with some standard following according to the already written e2e tests.

Major blocker in accepting this change is that this test assumes that an invoice is already available in the environment with the code IV-FCUL0001-171000001-2026-06 which will cause this test to always fail in the environment where this invoice doesn't exist. Approach in e2e tests should be to generated the related data for the test case before initiating the test rather than assuming it might already exist.

Comment thread cypress/e2e/invoice.cy.js Outdated
cy.enterMuiInput('Fees', payment.fees);
cy.enterMuiInput('Amount Received', payment.amountReceived);
cy.chooseMuiDatePicker('Payment Date', payment.paymentDate);
cy.enterMuiInput('Payment Origin', payment.amountReceived);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The form fills amountReceived twice and never tests paymentOrigin. E2E tests should test all the updated fields in assert.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

Comment thread cypress/e2e/invoice.cy.js Outdated
//invoice actions
const Invoice = {

goToList: () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These actions (goToList, goToForm, fillForm, verifyExists) should be defined as commands in commands.js to align with the structure used throughout the e2e tests.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I defined these actions in a specific file to the fe-invoice module (InvoicePage.js) because they are specific to the invoice module. For example, in the list view, there is no Add button since invoices are automatically generated by the system after a policy is assigned to a family.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Since invoice generation is handled by the system, I only have the family's CHFID to retrieve the invoice, This is why I had to create an openRow specific to fe-invoice, which ensures that the generated invoice code contains the family's CHFID

Comment thread cypress/e2e/invoice.cy.js Outdated
})

// verify delete
Invoice.goToList();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Navigation is duplicated across all 4 methods. goToList, goToForm, verifyExists, and delete all repeat the same "go to list → search by invoice code → open row" sequence.
This should be extracted as a command "goToInvoicePayments":

  cy.commands('goToList');
  cy.enterMuiInput('Code', data.invoice.code, 'input');
  cy.contains('button', 'Search').click({ force: true });
  cy.openRow(data.invoice.code);
  cy.contains(data.invoice.code).should('be.visible');
  cy.scrollTo('bottom');
  cy.contains('Payments').click();

All methods then call this command.

Comment thread cypress/e2e/invoice.cy.js Outdated
});
cy.contains('button', 'Search').click();
cy.get('table').should('be.visible');
cy.contains('td', data.payment.code)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

verifyExists is redundant with delete. delete calls verifyExists then repeats almost identical steps for the confirmation check. Can we extract the "navigate to payments and filter by code" part into a shared helper?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

adjusted in InvoicePage.js in support/pages repository

let normalizedMonth = month;
let normalizedYear = year;

if (day && typeof day === 'object') {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After the previous if/else, day is always a primitive. The inner block is unreachable:
// This entire block is dead code — you can remove it if (day && typeof day === 'object') { ... }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i've refactor the code in a new Datepicker command (chooseCraMuiDatePicker)

Comment thread cypress/e2e/invoice.cy.js Outdated
cy.contains('button', 'Search').click();
cy.get('table').should('be.visible');
cy.contains('td', payment.code)
.should('be.visible')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

After creating a payment, only payment.code is verified. A proper create test should assert that the saved record reflects the values that were submitted. Otherwise, the test cannot catch a form that submits but silently drops values.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

it's impossible to edit payment after creation
to ensure field was correctly saved i've implemented verifyRowValues method, to check fields values in row

Comment thread cypress/e2e/invoice.cy.js Outdated
cy.contains('button', 'Search').click();
cy.get('table').should('be.visible');
cy.contains('td', data.payment.code)
.should('not.be.visible')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 Wrong assertion for a deleted record — .should('not.be.visible') passes even if the element exists in the DOM but is hidden.
After deletion, the row should not exist at all.
Use: cy.contains('td', payment.code).should('not.exist');

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed in deletePayment method in InvoicePage.js

Comment thread cypress/e2e/invoice.cy.js Outdated
paymentOrigin: "epargne"
},
invoice: {
code: "IV-FCUL0001-171000001-2026-06"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Hardcoded invoice code will fail on any environment that doesn't have this exact record. This will fail in CI. Best approach is to ensure the data exists before testing. i.e. create this invoice in the test code and then use the code from the created invoice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

i've add the family and policy attribution process to generate the invoice

Comment thread package.json Outdated
},
"scripts": {
"cy:open": "cypress open"
"cy open": "cypress open",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Script names with spaces are not usable from the command line (npm run cy open will not work) and duplicate the colon-style scripts already present.
Remove the space-named entries. The correct scripts are already there:

"scripts": { "cy:open": "cypress open", "cy:run": "cypress run" }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

Comment thread cypress/e2e/invoice.cy.js Outdated
});

it('Create a new payment for invoice', () => {
cy.login();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Login belongs in beforeEach, not inside the test body.
This is the pattern used throughout the rest of the test suite. As more tests are added to this describe block, each one will need cy.login() manually. Move it up:

describe('Invoice payment workflow', () => { beforeEach(() => { cy.login(); }); // ... });

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

fixed

@vaneck237
Copy link
Copy Markdown
Author

I have added my review, there are some code refactoring suggestions along with some standard following according to the already written e2e tests.

Major blocker in accepting this change is that this test assumes that an invoice is already available in the environment with the code IV-FCUL0001-171000001-2026-06 which will cause this test to always fail in the environment where this invoice doesn't exist. Approach in e2e tests should be to generated the related data for the test case before initiating the test rather than assuming it might already exist.

I made a lot of changes based on your feedbak, could you please double check?

@vaneck237 vaneck237 changed the title add fe-invoice e2e tests Add invoice module E2E tests for payment management Apr 29, 2026
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