Solution by Julia2063 · Pull Request #137 · mate-academy/js_check_the_form

Julia2063 · 2025-12-06T00:17:36Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there is a critical ambiguity that directly affects the function’s correct use. Your tests consistently call validateRegisterForm(email, password), but the textual description also states that the function “takes password and email,” which implies the opposite order; this mismatch can easily lead to a wrong implementation even if all other logic is correct. You should choose one canonical parameter order (I recommend keeping validateRegisterForm(email, password) to match your current tests) and then:

Update the description file so all references and examples use that same order consistently.
Double-check the implementation to ensure its parameter list and internal logic match the chosen order.
Adjust any misleading test titles, e.g. rename the test whose title says "should return success message for the whole invalid input" to something like "should return error when both password and email are invalid" so it matches the expected code: 500 and error message.

Once these are aligned, the suite will clearly express the intended contract and your otherwise solid coverage of valid/invalid cases will be much easier to implement against. Overall the structure and scenarios of your tests are good; you just need to resolve this interface contract issue and the confusing test name before the work can be considered complete.

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

Important

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

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-12-06T00:22:33Z

src/validateRegisterForm.test.js


-  // write more tests here
+  it('should accept password with Cyrillic letters', () => {
+    const result = validateRegisterForm('test@mail.com', 'ПарольAa1!');


Tests consistently call validateRegisterForm with the email as the first argument (email, password) — for example here. Confirm the intended parameter order and make description, tests and implementation consistent. The description file currently contains conflicting references to parameter order, so align them.

mateacademy-ai-mentor · 2025-12-06T00:22:33Z

src/validateRegisterForm.test.js

+  it(`should return success message for the whole invalid input`, () => {
+    const isValid = validateRegisterForm('test@com', 'ssword1');
+
+    expect(isValid.code).toBe(500);
+    expect(isValid.message).toBe('Password and email are invalid.');


The test title here says "should return success message for the whole invalid input" but the assertions expect code: 500 and message 'Password and email are invalid.'. The title is misleading — rename to something like "should return error when both password and email are invalid" for clarity.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there is still a critical contract mismatch between the tests and the task description that will confuse implementers. In src/validateRegisterForm.test.js all tests call validateRegisterForm(email, password), but the task description still states the function signature as (password, email); this is a core requirement and must be consistent to avoid incorrect implementations. Please update the task description and any examples so that they clearly and consistently specify the same parameter order used in the tests (recommend validateRegisterForm(email, password)), or adjust the tests if you intentionally want the other order.

Beyond that blocker, the test suite is generally strong: it covers valid inputs, individual email/password failures, and the combined invalid case, and the previously confusing test name has been fixed. As minor, non-blocking improvements, consider adding: (1) a test for double dots in the domain part such as test@mail..com to fully enforce the "no double dots" rule, and (2) a positive email test that includes allowed special characters to show they pass validation. Once the signature inconsistency is resolved, your tests will be in good shape and fully aligned with the specification.

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

Important

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

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2025-12-06T01:11:40Z

src/validateRegisterForm.test.js

+    expect(invalidEmail.message).toBe('Email is invalid.');
+  });
+
+  it(`should return error when both password and email are invalid`, () => {


Nice: the test named "should return error when both password and email are invalid" clearly describes the expected 500 response for both-invalid inputs — this addresses the earlier feedback about a misleading title.

mateacademy-ai-mentor · 2025-12-06T01:11:40Z

src/validateRegisterForm.test.js

+
+  it(`should return error for email
+    with double dot in the first part of it and valid password`, () => {
+    const invalidEmail = validateRegisterForm(


You have a test for double dots in the local part ('test..test@mail.com') — consider adding one for double dots in the domain part (for example 'test@mail..com') to fully cover the requirement that "double dots are not allowed".

Julia2063 · 2025-12-06T01:49:47Z

Везде названия тестов я поменяла - порядок правильный - ошибки там нет!

mateacademy-ai-mentor

Review Summary

I am approving your solution as part of an educational override so you can keep progressing while you learn, but there are a couple of important points to fix next.

You implemented the core behavior very well: the function returns the correct status codes and messages for valid input, invalid email, invalid password, and both invalid, and your email regex and tests cover realistic edge cases like allowed special characters and double dots in the domain. The most critical issue is a contract mismatch between the description and the code/tests: your function and tests use validateRegisterForm(email, password), while the task description still says (password, email) — update the description/examples to match validateRegisterForm(email, password) so others don’t get confused. Also, your password regex currently checks only ASCII letters ([a-z] / [A-Z]), but the requirement mentions Cyrillic letters; decide whether Cyrillic-only passwords should satisfy the uppercase/lowercase rule, and if so, make the regex Unicode-aware (for example, using Unicode property escapes) and add a pure-Cyrillic success test. As a minor readability improvement, consider renaming variables like invalidEmail in tests that expect success to something like result or validEmailResult so the intent is clearer.

Overall, your structure, branching logic, and test coverage are strong; tightening the contract and Unicode handling will bring this to a very polished, real-world level.

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

Rate AI review example ↩

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.test.js

-  // write more tests here
+  it(`should return success message for valid email 
+    and password with Cyrillic letters`, () => {
+    const invalidPassword = validateRegisterForm('test@mail.com', 'ПарольAa1!');


Critical — parameter-order mismatch. All tests call validateRegisterForm(email, password), but the task description currently states the signature is (password, email). This is a blocking contract mismatch; update the description/examples to validateRegisterForm(email, password) (or change tests/implementation to match the description). See the description and tests for the current usages.

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.test.js

-  // write more tests here
+  it(`should return success message for valid email 
+    and password with Cyrillic letters`, () => {
+    const invalidPassword = validateRegisterForm('test@mail.com', 'ПарольAa1!');


Cyrillic acceptance test doesn't prove pure-Cyrillic support. The password on this line is a mix of Cyrillic and Latin letters (ПарольAa1!), so it doesn't confirm that a password using only Cyrillic letters will satisfy the uppercase/lowercase requirement. Add a test with a pure-Cyrillic valid password if you intend to accept Cyrillic uppercase/lowercase as fulfilling the rule, or clarify the requirement to restrict to Latin letters.

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.test.js

+
+  it(`should return success for email
+    with allowed characters and valid password`, () => {
+    const invalidEmail = validateRegisterForm('test!@mail.com', 'Password1!');


Variable name is confusing here: the test expects a success (200) but the variable is named invalidEmail. Consider renaming to something like result or validEmailResult to improve readability.

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.test.js

+
+  it(`should return error for email with double dots 
+    in the top Level domain and valid password`, () => {
+    const invalidEmail = validateRegisterForm('test@mail..com', 'Password1!');


Good: there is an explicit test for double dots in the top-level domain ('test@mail..com'), which enforces the 'no double dots' rule from the description — keep this.

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.js

 * @param {string} email
- *
 * @param {string} password
 *
 * @returns {object}
 */
 function validateRegisterForm(email, password) {


Contract mismatch (HIGH priority): The function signature in this file is validateRegisterForm(email, password) (JSDoc and parameter order at declaration). The task description/examples still state the signature as (password, email). This mismatch will confuse implementers and graders — update the task description/examples to validateRegisterForm(email, password) or change the implementation/tests to match the described order.

mateacademy-ai-mentor · 2025-12-06T01:54:14Z

src/validateRegisterForm.js

+  const validPassword
+    = /^(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[^a-zA-Z0-9])(?!.*\s).{8,16}$/;


Password regex may not accept Cyrillic as fulfilling uppercase/lowercase checks (MEDIUM): The regex uses (?=.*[a-z])(?=.*[A-Z]), which only matches ASCII letters. The task requires accepting letters Aa-Zz, Aa-Яя. If Cyrillic letters should count toward the uppercase/lowercase requirement, update the regex to be Unicode-aware (for example use \p{Ll}/\p{Lu} with the u flag or include Cyrillic ranges). If you intentionally require ASCII for those checks, document that limitation in the task description.

Solution

1a5d8c7

mateacademy-ai-mentor suggested changes Dec 6, 2025

View reviewed changes

Solution two

145e87f

Julia2063 requested a review from mateacademy-ai-mentor December 6, 2025 01:05

mateacademy-ai-mentor suggested changes Dec 6, 2025

View reviewed changes

Solution three

0b2aa71

Julia2063 requested a review from mateacademy-ai-mentor December 6, 2025 01:49

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

View reviewed changes

		const validPassword
		= /^(?=.\d)(?=.[a-z])(?=.[A-Z])(?=.[^a-zA-Z0-9])(?!.*\s).{8,16}$/;

Conversation

Julia2063 commented Dec 6, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Julia2063 commented Dec 6, 2025

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants