Skip to content

add first task solution#1921

Open
shyshenok81-hub wants to merge 3 commits intomate-academy:masterfrom
shyshenok81-hub:develop
Open

add first task solution#1921
shyshenok81-hub wants to merge 3 commits intomate-academy:masterfrom
shyshenok81-hub:develop

Conversation

@shyshenok81-hub
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the employee object schema in src/scripts/main.js still does not match the task requirements and this will cause the tests to fail. Specifically, in getEmployees, you are using both the variable and the returned object key as employeeName instead of the required name. The task requires each employee object to have the shape { name, position, salary, age }, so you need to: (1) rename the extracted variable from the DOM to name (e.g., const name = item.textContent.split('\n')[0].trim() || ''), and (2) return { name, position, salary, age } from getEmployees instead of using the employeeName key. This schema mismatch is a critical blocker because any automated checks or consumers expecting the name property will not work correctly. Aside from this, your parseSalary, sortList, and overall logic for sorting and building the employees array look solid. Once you align the variable and object property to name, your solution should fully meet the task requirements.


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example


function getEmployees(listElement) {
return [...listElement.children].map((item) => {
const employeeName = item.textContent.split('\n')[0].trim() || '';

Choose a reason for hiding this comment

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

The variable is named employeeName but the task requires the property to be name. Rename this variable to name, e.g. const name = item.textContent.split('\n')[0].trim() || ''; so the object matches the schema.

const age = Number(item.dataset.age) || null;

return {
employeeName,

Choose a reason for hiding this comment

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

The returned object uses the employeeName key. Return { name, position, salary, age } (use name as the property) to satisfy the required employee schema.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Great job addressing the previous feedback on the employee object schema! Your solution is very close, but some changes are needed as the automated tests are currently failing. Here are the two issues to address:

  1. Incorrect Name Parsing: In your getEmployees function, the logic to extract an employee's name (item.textContent.split('\n')[0].trim()) is returning an empty string because of whitespace in the HTML. This is the primary cause of the test failure. A more reliable way to get the name is to access the first child node's text content directly, like this: const nameText = item.firstChild.textContent.trim();.

  2. Redundant Function Call: The getEmployees(list); call at the end of your script is unnecessary because its return value is not used. Please remove this line to avoid potential side effects in the testing environment.

After fixing these two points, your solution should pass the checks. You're almost there!


✨ Please rate this review in the Platform's notification center1. 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 button2 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 tag3.
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.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Sorry, the pull request review was unsuccessful for the following reasons:

Warning

No changes were made in the pull request since the last review. Please make sure that you have added files to the pull request and committed the changes.
[CODE: NCM]

If the issue is within your control, please address it.
The next review attempt will automatically launch in about 1 hour.
If you believe this is an error or need assistance, please contact the support team.

Copy link

@Anton-Kuchmasov Anton-Kuchmasov left a comment

Choose a reason for hiding this comment

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

Please make fixes from following thread in fe_chat:
https://mate.academy/chats/20895?selectedMessageId=10311127

Copy link

@vadiimvooo vadiimvooo left a comment

Choose a reason for hiding this comment

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

Well done

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.

4 participants