Main task is done #8

Bulavskyi · 2025-01-06T07:24:27Z

No description provided.

mateacademy-ai-mentor

Thank you for your hard work on this pull request! 😊 While you've done a great job overall, there are a few critical issues that need to be addressed before we can approve it:

Syntax Errors: There are several missing semicolons across different files (BankHomePage, AddCustomerPage, BankManagerMainPage, and some test scripts). These could lead to syntax errors, which are critical as they can break the functionality of your code.
Awaiting Method Calls: The method clickCustomersTab() should be awaited to ensure the correct sequence of actions. This is crucial to maintain the functionality of the test script.
Test Descriptions and Comments: Some test descriptions and comments are misleading or incorrect. It's important to update these to accurately reflect the actions being tested to avoid confusion.
Import Statements: The import statement for Faker is unnecessary if only faker is being used. Removing unused imports helps keep the code clean and maintainable.

Please address these issues to ensure the functionality and clarity of your code. Once these are resolved, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

mateacademy-ai-mentor · 2025-01-06T07:27:29Z

src/pages/BankHomePage.js

@@ -4,6 +4,7 @@ export class BankHomePage {
  constructor(page) {
    this.page = page; 
    this.customerLoginButton = page.getByRole('button', { name: 'Customer Login' });
+    this.managerLoginButton = page.getByRole('button', { name: 'Bank Manager Login' })


There is a missing semicolon at the end of this line. It's important to include semicolons to avoid potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:27:29Z

src/pages/manager/AddCustomerPage.js

+    this.postCodeField = page.getByPlaceholder('Post Code');
+    this.addCustomerButton = page.getByRole('form').getByRole('button', { name: 'Add Customer' });
+
+    this.accountTab = page.getByRole('button', { name: 'Open Account' })


There is a missing semicolon at the end of this line. It's important to include semicolons to avoid potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:27:29Z

src/pages/manager/BankManagerMainPage.js

+  }
+
+  async getButtonByName (buttonName) {
+    return this.page.getByRole('button', { name: `${buttonName}` })


There is a missing semicolon at the end of this line. It's important to include semicolons to avoid potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:27:29Z

tests/manager/addCustomer/managerCanAddNewCustomer.spec.js

@@ -1,5 +1,7 @@
 import { test } from '@playwright/test';
-import { faker } from '@faker-js/faker';
+import { Faker, faker } from '@faker-js/faker';


The import statement for Faker is unnecessary if you are only using faker. You can remove Faker from the import.

mateacademy-ai-mentor · 2025-01-06T07:27:29Z

tests/manager/addCustomer/managerCanAddNewCustomer.spec.js

+10. As customer in the table in the last row. 
+sert the Postal Code of the customer is present in the table in the last row. 
+11. Assert there is no account number for the new


There seems to be a formatting issue in the test description comments. Ensure that the steps are clearly numbered and described. It looks like step 10 is split across two lines, and step 11 is incomplete.

mateacademy-ai-mentor · 2025-01-06T07:27:30Z

tests/manager/openAccount/managerCanChooseCurrencies.spec.js

+    await openAccountPage.assertCurrency('Dollar')
+
+    await openAccountPage.selectCurrency('Pound');
+    await openAccountPage.assertCurrency('Pound')


There is a missing semicolon at the end of this line. It's important to include semicolons to avoid potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:27:30Z

tests/manager/openAccount/managerCanChooseCurrencies.spec.js

+    await openAccountPage.assertCurrency('Pound')
+
+    await openAccountPage.selectCurrency('Rupee');
+    await openAccountPage.assertCurrency('Rupee')


There is a missing semicolon at the end of this line. It's important to include semicolons to avoid potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:27:30Z

tests/manager/openAccount/managerCanOpenAccount.spec.js

+  await addCustomerPage.fillFirstNameField(customer.firstName);
+  await addCustomerPage.fillLastNameField(customer.lastName);
+  await addCustomerPage.fillPostCode(customer.postCode);
+  await addCustomerPage.clickAddCustomerButton();
 });

 test('Assert manager can add new customer', async ({ page }) => {


The test name 'Assert manager can add new customer' is misleading. It should reflect the action being tested, such as 'Assert manager can open a new account'.

mateacademy-ai-mentor · 2025-01-06T07:27:30Z

tests/manager/searchCustomer/managerCanSearchCustomerByFirstName.spec.js

+
+  await customersListPage.searchForCustomer(customer.firstName);
+  await customersListPage.assertCustomerRowVisibility(customer.firstName, customer.lastName, customer.postCode, true);
+  await customersListPage.assertCustomerRowCount(2); // 1-header row with text


The comment suggests that the expected row count is 2, including the header row. Ensure that this matches the actual structure of your table. If the table includes a header row, the expected count should be 2; otherwise, it should be 1.

mateacademy-ai-mentor · 2025-01-06T07:27:30Z

tests/manager/searchCustomer/managerCanSearchCustomerByLastName.spec.js


 });

 test('Assert manager can search customer by Last Name', async ({ page }) => {
 /* 
 Test:
 1. Open Customers page
-2. Fill the lastName to the search field
+2. Fill the firstName to the search field


The comment mentions filling the 'firstName' in the search field, but the test is for searching by 'lastName'. Update the comment to accurately reflect the action: 'Fill the lastName to the search field'.

mateacademy-ai-mentor

Great job on your submission! 🎉 While your code is mostly well-implemented, there are a few areas that need attention to ensure clarity and functionality:

Syntax Errors: Ensure all lines, such as those in BankHomePage.js and AddCustomerPage.js, have the necessary semicolons to prevent potential syntax errors.
Awaiting Method Calls: It's crucial to await method calls like clickCustomersTab() in your test scripts to maintain the correct sequence of actions.
Test Descriptions and Comments: Some test descriptions and comments are misleading or incomplete. For instance, in managerCanAddNewCustomer.spec.js, ensure the description accurately reflects the test case, and complete any unfinished comments.
Unused Imports: Remove any unused imports, such as Faker, to keep your code clean and efficient.
Assertion Clarity: Make sure assertions, like those for customer row counts, clearly explain the expected outcomes, especially if they include header rows.

By addressing these issues, you'll enhance the clarity and functionality of your code. Keep up the great work, and continue learning from these experiences! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

src/pages/BankHomePage.js


 export class BankHomePage {
  constructor(page) {
    this.page = page; 
    this.customerLoginButton = page.getByRole('button', { name: 'Customer Login' });
+    this.managerLoginButton = page.getByRole('button', { name: 'Bank Manager Login' })


There is a missing semicolon at the end of this line. Adding it will prevent potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

src/pages/manager/AddCustomerPage.js

+    this.postCodeField = page.getByPlaceholder('Post Code');
+    this.addCustomerButton = page.getByRole('form').getByRole('button', { name: 'Add Customer' });
+
+    this.accountTab = page.getByRole('button', { name: 'Open Account' })


There is a missing semicolon at the end of this line. Adding it will prevent potential syntax errors.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/addCustomer/managerCanAddNewCustomer.spec.js


-test('Assert manager can add new customer', async ({ page }) => {
+test('Assert manager can open new account', async ({ page }) => {


The test description is misleading. It should reflect the actual test case, which is about adding a new customer, not opening a new account.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/addCustomer/managerCanAddNewCustomer.spec.js

+10. As customer in the table in the last row. 
+sert the Postal Code of the customer is present in the table in the last row. 
+11. Assert there is no account number for the new


There are typos in the comments. Line 18 should start with 'Assert the Postal Code of the customer is present in the table in the last row.' and line 20 should complete the sentence about asserting there is no account number for the new customer.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/openAccount/managerCanOpenAccount.spec.js

+  await addCustomerPage.fillFirstNameField(customer.firstName);
+  await addCustomerPage.fillLastNameField(customer.lastName);
+  await addCustomerPage.fillPostCode(customer.postCode);
+  await addCustomerPage.clickAddCustomerButton();
 });

 test('Assert manager can add new customer', async ({ page }) => {


The test description is misleading. It should reflect the actual test case, which is about opening a new account, not adding a new customer.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/searchCustomer/managerCanSearchCustomerByFirstName.spec.js

+
+  await customersListPage.searchForCustomer(customer.firstName);
+  await customersListPage.assertCustomerRowVisibility(customer.firstName, customer.lastName, customer.postCode, true);
+  await customersListPage.assertCustomerRowCount(2); // 1-header row with text


The assertion for the customer row count should clearly indicate why the expected count is 2. If the count includes a header row, ensure this is clearly documented in the test steps or comments.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/searchCustomer/managerCanSearchCustomerByLastName.spec.js


+  await customersListPage.searchForCustomer(customer.lastName);
+  await customersListPage.assertCustomerRowVisibility(customer.firstName, customer.lastName, customer.postCode, true);
+  await customersListPage.assertCustomerRowCount(2); // 1-header row with text


The assertion for the customer row count should clearly indicate why the expected count is 2. If the count includes a header row, ensure this is clearly documented in the test steps or comments.

mateacademy-ai-mentor · 2025-01-06T07:39:56Z

tests/manager/searchCustomer/managerCanSearchCustomerByPostalCode.spec.js


+  await customersListPage.searchForCustomer(customer.postCode);
+  await customersListPage.assertCustomerRowVisibility(customer.firstName, customer.lastName, customer.postCode, true);
+  await customersListPage.assertCustomerRowCount(2); // 1-header row with text


The assertion for the customer row count should clearly indicate why the expected count is 2. If the count includes a header row, ensure this is clearly documented in the test steps or comments.

main task done

934ada2

mateacademy-ai-mentor suggested changes Jan 6, 2025

View reviewed changes

small fixes

9f502f9

Bulavskyi requested a review from mateacademy-ai-mentor January 6, 2025 07:37

mateacademy-ai-mentor approved these changes Jan 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Main task is done #8

Main task is done #8

Bulavskyi commented Jan 6, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor left a comment

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025

mateacademy-ai-mentor Jan 6, 2025


		test('Assert manager can add new customer', async ({ page }) => {
		test('Assert manager can open new account', async ({ page }) => {

Main task is done #8

Are you sure you want to change the base?

Main task is done #8

Conversation

Bulavskyi commented Jan 6, 2025

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment