236 add hash to course unit and method to verify hash

[jose-carlos-sousa]

Closes #236
Added hash to the types
Added func to get hashes
Created hook to verify hashes against localStorage

[netlify]

✅ Deploy Preview for tts-fe-preview ready!

Name	Link
🔨 Latest commit	11f842d
🔍 Latest deploy log	https://app.netlify.com/sites/tts-fe-preview/deploys/66ce4457207d0a0008c8445e
😎 Deploy Preview	https://deploy-preview-250--tts-fe-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[tomaspalma]

Great work!

Maybe you can also use the hook inside the CoursesController (this is the component that renders the dropdowns to select the classes) to actually perform the fetching of the hashes. And then in order to see if there are invalid classes, maybe a useEffect that runs when the data from the useSwr returned changes so that we can perform an action when there are invalid courses. The current decided action is to re-fetch the classes from that course unit.

If you think that should be done in a different component, feel free to say so!

I actually forgot to put that in the issue description that it was also the scope of this issue, I'm sorry for that.

[thePeras]

I am not a big fan of using periodic check, an initial check in a useEffect would work fine.

In the code, I suggest moving updateCourses function outside the useEffect so it won't be declared multiple times

[tomaspalma]

I am not a big fan of using periodic check, an initial check in a useEffect would work fine.

Since we are using useSwr, regardless of whether or not we make periodic requests, we don't need a useEffect because regardless of the refresh interval, every time you refresh the page it will make the request and update the data if it has changed.

Although I agree that periodic requests are not strictly necessary, there isn't a lot of harm to it, also because the amount of people making the same periodic request at the same time will be very low and it will be between 5-10 minutes.

Is the reason why you don't like periodic requests the potential unecessary increase in the number of requests?

In the code, I suggest moving updateCourses function outside the useEffect so it won't be declared multiple times

I also agree with that, it is a good idea!

[thePeras]

Is the reason why you don't like periodic requests the potential unecessary increase in the number of requests?

Yes! Imagine we implemented the periodic scrapper today, we would probably make it run at 3h00, so, a periodic 5/10 min checker is something irrelevant since 0 users would be using the tts at that time. When the person visits the page and useEffect is triggered, it is enough to know the data is updated.
I think sleeping window tabs runs useEffect when they become active. But this needs to be confirmed! Because a user can have the tab indefinably in the browser.

If requesting this information when refresh the page is a problem, we can have some local storage variable like "have fetched today?"

[tomaspalma]

If the scrapper runs at 3 am for example one time per day, yes, there is no need for periodic checking. However, the period in which the scrapper will run also depends on capabilities of niployments.

However, regardless of that, as you pointed out, the scrapper will not run too frequently probably, so the use cases where a periodic request is useful are low.

I dont think there is a problem in fetching the data when we refresh the page.

[jose-carlos-sousa]

There is an issue where the app crashes when a class that I have selected is removed one solution could be to unselect the class of a course unit that has been changed

[tomaspalma]

Can you please put here the exact error message it gives on the console?

Unfortunately, I'm unable to recreate the issue, I altered the hash on the backend on a course_unit that I had chosen a class (1LEIC01) on the frontend and no crash occured when the hash was changed.

[jose-carlos-sousa]

Can you please put here the exact error message it gives on the console?

Unfortunately, I'm unable to recreate the issue, I altered the hash on the backend on a course_unit that I had chosen a class (1LEIC01) on the frontend and no crash occured when the hash was changed.

I basically used the same scenario to test and I got an error at the function getAllPickedSlots (index.ts) because classInfo was undefined so I added if( !classInfo ) return []; I will push

[jose-carlos-sousa]

I realized that the app crashes when I pick a class and then the class I picked is removed

Can you please put here the exact error message it gives on the console?

Unfortunately, I'm unable to recreate the issue, I altered the hash on the backend on a course_unit that I had chosen a class (1LEIC01) on the frontend and no crash occured when the hash was changed.

ps the scenario I used was to start with normal db and select 1LEIC01 for FP then change to db where hash is changed and where there is no class 1LEIC01 for course unit FP and it crashed

[tomaspalma]

Ok, so just to clarify, that error appears jf the class you had selected was removed, right?

I hadnt thought of that scenario, it is a good observation

[jose-carlos-sousa]

Ok, so just to clarify, that error appears jf the class you had selected was removed, right?

I hadnt thought of that scenario, it is a good observation

Yes that's it ,it appeared because 1LEIC01 was removed and it was the selected one
Currently I use that line if( !classInfo ) return []; that basically unselects the class when that happens which at least is better than crashing but I am not really sure about other solutions yet

[tomaspalma]

It is not usual for classes to disappear. Normally the suddden data changes in sigarra are changes in the schedules or in the teachers, so that is a really narrow edge case.

I think just disappearing is fine for now, but the other solution would be to say to the user that the class disappeared in the refresh data, but I dont know if that is really needed

[tomaspalma]

🚀

a detail came up

[tomaspalma]

🚀 (not going to dimiss the review this time xD)

[thePeras]

Very clean and documented code! Nice work!

✅ 🛫