theHocineSaad / linatabara3

Linatabara3 is an open source Laravel 9 blood donation platform that connects blood seekers with blood donors in their region.
https://linatabara3.com
MIT License
78 stars 12 forks source link

refactor code && fix #5

Closed haddadzineddine closed 2 years ago

haddadzineddine commented 2 years ago

1- refactor the code inside seeders 2- fix wilaya name in user update 3- delete wilaya observable && user observable ( no need to them ) , we can't change them at code-level

theHocineSaad commented 2 years ago

Thank you @haddadzineddine for contributing. Can I know why you deleted Wilaya's and User's observers ?

haddadzineddine commented 2 years ago

Well, usually we use them to watch changes in the model (so when you delete or create ... u can trigger them before or after you persist the data into the database),

But in our case, we can't change wilaya table at the code-level (same to the user) because there is no create or update.. handler in your project.

Also, I just saw in the user observe that u used 'stats' key for the cache? I think you mean bloodGroup observer, not user observer,

Anyway, we don't need them anymore.

hope it's clear now , if it's not tell me plz /=

theHocineSaad commented 2 years ago

We're using Cache for Wilayas and Users, so we need to clear it every time we add or delete a Wilaya or User.

For stats (the section that shows the number of donors of each blood group) we get all the users from the database and this is very expensive and will slowdown the website when the number of donors will be big so I cached it, so the User observer is necessary to clear the cache every time a new donor is registered, otherwise we will get old data which means wrong stats numbers. The 'stat' key in User observer is correct, we clear the 'stat' cache on user signup, delete...

For Wilaya observer, it's the same thing, it's because we cache Wilayas, I added it because in future I may add an admin panel where the owner of the website can do the CRUD on Wilaya modal, I know adding new Wilayas is not a thing we do everyday, but if it happened, the observer will be there.

So, the user observer is necessary, please redo it. The Wilaya observer is to make the project future proof, I let you choose what to do with it.

haddadzineddine commented 2 years ago

i see , but there's no place in the code where we get all users ( donors ), the only place is in the search and we can't cache at this step;

for Wilaya , i prefer to don't add things that we don't use right away ;

theHocineSaad commented 2 years ago

For Wilaya observer, okay let's remove it.

For User observer, in app/View/Components/Stats.php we have this line of code: return BloodGroup::withCount('users')->get(); this line will convert to: select (select count(*) from 'users' where 'blood_groups'.'id' = 'users'.'blood_group_id') as 'users_count' from 'blood_groups' We are selecting from users to get the number of users for each blood group, and we are caching the result, so if we don't clear cache every time a new user is registered, we will show old result, wrong stats. please correct me if I am wrong.

I think it's best to make cache names and class attributes like that:

haddadzineddine commented 2 years ago

check the new updates 👍🏻