Skip to content

Conversation

@charlietharas
Copy link
Contributor

@charlietharas charlietharas commented Jan 8, 2025

@mlaws21 trying to replace localStorage.getItem and localStorage.setItem calls in reducers/course.js with wrappers to the API calls (wso.courseSchedulerService.setSelection() etc.) but I don't know if it is possible to refactor this without moving stuff to React hooks/components (and don't know the proper way to do so)

@charlietharas charlietharas requested a review from mlaws21 January 8, 2025 17:18
@mlaws21
Copy link
Contributor

mlaws21 commented Jan 27, 2025

What doesn't work -- if you just replace the localStorage.getItem("added"); with the api call what happens? I feel like that should work... you will also have to make the function async and await and stuff but I dont see why that wouldn't work -- you also would have to update it throughout the file.

You can walk me through it at some point, and we can try to debug

@charlietharas
Copy link
Contributor Author

In order to access the object we use to call the API, I need to be in a React hook/component, and I couldn't find a neat way to do that--not like I can just call the API directly with async fetch

@mlaws21
Copy link
Contributor

mlaws21 commented Jan 29, 2025

Ok that makes sense, I don't exactly know what to do, not super experienced with redux but I think that redux-thunk is the right way to do this...

https://redux.js.org/tutorials/essentials/part-5-async-logic

if you lmk the API calls you are actually trying to make I can try to hook everything up, but your try is as good as mine.

@charlietharas
Copy link
Contributor Author

I think it's

await wso.courseScheduler.getSelections(userID)
await wso.courseScheduler.setSelections(userID, {courses: "addedCourses"})

TODO for myself: this code is horrible! Add the Redis client as a parameter to the CourseSchedulerController, rename unixID to userID, go through the rest and see what other stupid stuff I have done.

@charlietharas
Copy link
Contributor Author

Could also be wso.CourseSchedulerService or some other capitalization

@charlietharas
Copy link
Contributor Author

Will need to also add API sync code to handle major builder state--so holding off on this PR until I go through the backend and API code and clean up my mess. Steps for this whole process:

  • Go through backend Redis code; clean up; ask for forgiveness from the powers that be
  • Add new Redis code to the backend to also sync major builder state
  • Update API client
  • Add sync hooks to both scheduler/timetable and major builder

This will be quite a headache.

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.

3 participants