juninhokaponne / friendRecomendation

This project aims to use a simple recommendation algorithm, initially it was made for a challenge and today it counts as a free repository for everyone to contribute.
3 stars 5 forks source link

Can we refractor this part or Not #6

Closed manzil-infinity180 closed 8 months ago

manzil-infinity180 commented 1 year ago

from src/controller/PersonController.js line - 9

if (!cpf || cpf.toString().length !== 11) {
      throw new AppError("CPF must contain exactly 11 numeric digits.", 400);
    }

as we already have in - src/middleware/validateCPF.js

juninhokaponne commented 1 year ago

Sure, I'll assign this issue for you.

manzil-infinity180 commented 1 year ago

sir can you help me with this part as i thought that i can remove that lines of code then this will work but it did'nt only 9 cases passed out of 10

manzil-infinity180 commented 1 year ago

can we use this -

router.post("/person", personController.createPersonRoute);

on the place of

router.post("/person",validateCPF, personController.createPersonRoute);

in that case it passes all the cases

juninhokaponne commented 1 year ago

This error occurs because there is a test case validation that validates an error message in the case of invalid cpfs, I simulated the error you are probably having.

If you're using this first one example, like that :

router.post("/person", personController.createPersonRoute);

It's okay. Just do a small test battery creating some users with invalid cpfs to confirm the functionality.

manzil-infinity180 commented 1 year ago

This error occurs because there is a test case validation that validates an error message in the case of invalid cpfs, I simulated the error you are probably having.

If you're using this first one example, like that :

router.post("/person", personController.createPersonRoute);

It's okay. Just do a small test battery creating some users with invalid cpfs to confirm the functionality.

ok i will perform some example then after the confirmation i will raise a PR . thank u sir ❤️

juninhokaponne commented 1 year ago

I'm here if you need help. Thank you for continuing to contribute to this.

juninhokaponne commented 1 year ago

@manzil-infinity180 It's everything okay?

After your PR and a few others, I will send a new release with all these features. Thanks

manzil-infinity180 commented 1 year ago

@manzil-infinity180 It's everything okay?

After your PR and a few others, I will send a new release with all these features. Thanks

@manzil-infinity180 It's everything okay?

After your PR and a few others, I will send a new release with all these features. Thanks

OK sir I will contribute to this repo currently I have my minors exam that why didn't give to much time but after some day I will try my best to contribute and add new features... Thank u sir ❤️

Akshay02022 commented 1 year ago

This error occurs because there is a test case validation that validates an error message in the case of invalid cpfs, I simulated the error you are probably having.

If you're using this first one example, like that :

router.post("/person", personController.createPersonRoute);

It's okay. Just do a small test battery creating some users with invalid cpfs to confirm the functionality.

sir can you help me with this part as i thought that i can remove that lines of code then this will work but it did'nt only 9 cases passed out of 10

I believe I've identified why the test is failing, and it's essential to understand understand what we are trying to achieve. We aim to remove the validation code that checks if a CPF (Brazilian individual tax ID) has exactly 11 numeric digits from the PersonController. This validation is now handled by the validateCPF middleware, which is a good practice.

However, when we run the tests in person.test.js, they fail because we are directly testing if "123" will pass or not. image Since we've removed the validation lines in the PersonController, "123" will indeed pass, and the test case will succeed.

To resolve this, we need to find a way to test the (!cpf || cpf.toString().length !== 11) condition through the middleware in person.test.js. This way, we can ensure that the middleware correctly throws an error when it encounters an invalid CPF, and our test case will pass as expected.

Another option is to get rid of the middleware and instead include the validation directly within the PersonController, which is the preferred approach for this project. This means we won't be using middleware anywhere else in the project.

I'm sharing this to explain why the test is currently failing, and this information might be helpful in the future.

manzil-infinity180 commented 1 year ago

This error occurs because there is a test case validation that validates an error message in the case of invalid cpfs, I simulated the error you are probably having. If you're using this first one example, like that :

router.post("/person", personController.createPersonRoute);

It's okay. Just do a small test battery creating some users with invalid cpfs to confirm the functionality.

sir can you help me with this part as i thought that i can remove that lines of code then this will work but it did'nt only 9 cases passed out of 10

I believe I've identified why the test is failing, and it's essential to understand understand what we are trying to achieve. We aim to remove the validation code that checks if a CPF (Brazilian individual tax ID) has exactly 11 numeric digits from the PersonController. This validation is now handled by the validateCPF middleware, which is a good practice.

However, when we run the tests in person.test.js, they fail because we are directly testing if "123" will pass or not. image Since we've removed the validation lines in the PersonController, "123" will indeed pass, and the test case will succeed.

To resolve this, we need to find a way to test the (!cpf || cpf.toString().length !== 11) condition through the middleware in person.test.js. This way, we can ensure that the middleware correctly throws an error when it encounters an invalid CPF, and our test case will pass as expected.

Another option is to get rid of the middleware and instead include the validation directly within the PersonController, which is the preferred approach for this project. This means we won't be using middleware anywhere else in the project.

I'm sharing this to explain why the test is currently failing, and this information might be helpful in the future.

really appreciating your explanation @Akshay02022 sir

juninhokaponne commented 1 year ago

I believe removing the middleware might be a good choice if done for now.

Akshay02022 commented 1 year ago

Indeed, you can go ahead and delete the middleware folder directly from the project directory as it's not necessary for the project.