Skip to content
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

Fix event following bug by fetching follow-status through event object #3541

Merged
merged 1 commit into from Feb 10, 2023

Conversation

eikhr
Copy link
Member

@eikhr eikhr commented Feb 9, 2023

Description

Depends on webkom/lego#3214

There is a bug where the webapp does not correctly load the following status of the current user if an event has a large number of followers. It is caused by pagination on the event follower endpoint, but I figured a better solution than loading all pages in the webapp was to just include the single relevant follow in the event object.

Result

The bug is no longer there.

Testing

  • I have thoroughly tested my changes.

I have tried following and unfollowing events multiple times. I have also tested that event detail page still works fine in logged out state.


Resolves webkom/lego#2521

Copy link
Member

@LudvigHz LudvigHz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, it's kind of a hack but if it works it works.

Is following a parameter on the event object from the event detail API?

@eikhr
Copy link
Member Author

eikhr commented Feb 10, 2023

Yup, I added "following" as a field on EventReadUserDetailedSerializer in the backend.

I don't see it as a very hacky solution. Instead of loading every user who is following the event we just include the relevant following information directly in the event.

@LudvigHz
Copy link
Member

Yup, I added "following" as a field on EventReadUserDetailedSerializer in the backend.

I don't see it as a very hacky solution. Instead of loading every user who is following the event we just include the relevant following information directly in the event.

Ah, okay I didn't see that 🙃. Makes sense 👍

Copy link
Member

@ivarnakken ivarnakken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This looks solid!

@ivarnakken ivarnakken added bug-fix Pull requests that fix a bug approved Pull requests that have been approved and removed review-needed Pull requests that need review labels Feb 10, 2023
@github-actions github-actions bot added the review-needed Pull requests that need review label Feb 10, 2023
@eikhr eikhr enabled auto-merge February 10, 2023 12:48
@eikhr eikhr removed the review-needed Pull requests that need review label Feb 10, 2023
@eikhr eikhr merged commit f648212 into master Feb 10, 2023
@eikhr eikhr deleted the following-bug branch February 10, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug
Projects
None yet
3 participants