Skip to content

bug: Mark as completed not working as intended #1794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Dhaneshwarguiyan opened this issue Mar 26, 2025 · 11 comments · May be fixed by #1796
Open

bug: Mark as completed not working as intended #1794

Dhaneshwarguiyan opened this issue Mar 26, 2025 · 11 comments · May be fixed by #1796
Labels
bug Something isn't working

Comments

@Dhaneshwarguiyan
Copy link

Describe the bug
Mark as completed not working as intended the state of markascompleted resets to original state after the sidebar panel is closed

To Reproduce
Steps to reproduce the behavior:

  1. Open Course content sidebar
  2. Mark any video as completed, then wait for the loader.
  3. Close the sidebar and then reopen
  4. The checkbox remains in its original state.
  5. It needs to be refreshed for the changes to reflect.
    A clear and concise description of what you expected to happen.
    It should be marked as completed even though I closed the sidebar and reopened.

Screenshots or GIFs
https://github.com/user-attachments/assets/e7d8f59d-380c-435d-a604-17d4f23a43ce

Info (please complete the following information):

  • Browser Chromium
  • Version 21

Additional context
It updates the db but does not fetches the latest data when the sidebar is closed and reopened,so state gains its default value and data is not fetched even though state has changed because of the useCallback hook which caches the handler(mark as completed).

@Dhaneshwarguiyan Dhaneshwarguiyan added the bug Something isn't working label Mar 26, 2025
@Dhaneshwarguiyan
Copy link
Author

Screencast.from.26-03-25.10.58.06.AM.IST.webm

@HarshK200
Copy link

The problem seems to be happening because all the content is server side rendered in a react server component, so when you check a video as completed it updates it in the db but doesn't re-fetch the data.

Possible Solutions:

  • One way to solve this would be just re-fetch the data every single time a video is marked as check or unchecked(but that would make to many fetch calls even if you were to put de-bouncing)

  • A better way would be to just not make these server side rendered and instead make an AtomFamily (redux state) for the data that would be fetched on the first render of courses/[id] page.

**NOTE: this would also fix the duplicate data fetching on the same page in the layout.tsx and page.tsx files as pointed in the image below**

Image

Also since the state would be centralized it would make the side-effect much more easier like for eg: each of the video card also get's a check mark when you check a video but that is impossible unless you reload the whole page to get the updated content after a video is checked completed (pointed in the image below).

Image

If you're fine with it @devsargam i would like to work on implementing the second approach.

@devsargam
Copy link
Collaborator

No, we can't do that client side at all. It will just leak all our db and api calls.

@devsargam
Copy link
Collaborator

Maybe we can revalidate the cache once someone presses the mark as complete action

@HarshK200
Copy link

@devsargam What i meant my making it client side is to make a route on the api that the client can hit and api route would return courseData depending on the id passed as query parameter (the db call still happens on the server the client got no idea)

And as for leaking api calls it would be a similar call to this which is already in prod (these are probably restricted by CORS so no problem i guess?)

Image

One downside: would be you can't have statically generated dynamic routes pages just sitting on the server that the server can just return on a request

If the above downside is too much then re-validating the cache would make sense but still handling side effect would still be pretty much impossible without a full reload or re-render

@Dhaneshwarguiyan
Copy link
Author

Maybe we can revalidate the cache once someone presses the mark as complete action

I have done the same created a server component and revalidated the path

@Dhaneshwarguiyan
Copy link
Author

Dhaneshwarguiyan commented Mar 27, 2025

I checked it for the bookmark action it does the same thing so I created a server side component to revalidate any path from client side and called this component in the check component @devsargam

@Dhaneshwarguiyan
Copy link
Author

Dhaneshwarguiyan commented Mar 27, 2025

Image
Is this fine or should we create a hook to do the same,cause there is a separate hook for bookmark useBookmark to mark or unmark the bookmark but we have to create separate hooks for each revalidation

@HarshK200
Copy link

@Dhaneshwarguiyan The useBookmark() hook also has an issue with the sidebar and cardView i've mentioned it here: #1795

@Dhaneshwarguiyan
Copy link
Author

@HarshK200 That is happening probably because bookmark component is dependent on addedBookmark state,
sidebar and contentcard has their own different states,when sidebar bookmark is toggled it updates its own state and database and at the same time due to revalidatePath updated data is fetched from the database but state is not updated,

Image

const [addedBookmark,setAddedBookmark] = useState(bookmark);
this line runs only once during the first render and hence addedBookmark is not updated with newly fetched data.
Using a useEffect hook will fix this.

@Dhaneshwarguiyan
Copy link
Author

@devsargam There are some linting error in code HammerInput not defined when I run lint:fix and lint:check,what to do?

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants