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

Show events from user communities on dashboard #2026

Merged
merged 13 commits into from
Oct 2, 2021

Conversation

darrenvong
Copy link
Member

@darrenvong darrenvong commented Sep 28, 2021

Probably my first time getting my hands dirty on the backend a little bit on this project, so hopefully I haven't done anything too terrible :p it was too inefficient and annoying me to have to send one request per community just to find out which events are in the communities the user is in, so I extended the ListMyEvents rpc to include events from the user's communities by default as well.

Desktop
Screenshot 2021-09-28 at 02 21 33

Mobile
Screenshot 2021-09-28 at 02 22 07

Closes #1880

Backend checklist

  • Formatted my code by running autoflake -r -i --remove-all-unused-imports src && isort . && black . in app/backend
  • Added tests for any new code or added a regression test if fixing a bug
  • All tests pass - I'm not sure why the migration tests are failing for me locally in Docker, but it's fine in GitLab??
  • Run the backend locally and it works
  • Added migrations if there are any database changes, rebased onto develop if necessary for linear migration history

Web frontend checklist

  • Formatted my code with yarn format && yarn lint --fix
  • There are no warnings from yarn lint
  • There are no console warnings when running the app
  • Added any new components to storybook
  • Added tests where relevant
  • All tests pass
  • Clicked around my changes running locally and it works
  • Checked Desktop, Mobile and Tablet screen sizes

@darrenvong darrenvong added 1.topic backend This issue relates to the python backend 1.topic frontend release notes: pending Add to stuff that should be included in release notes labels Sep 28, 2021
@darrenvong darrenvong requested review from a team September 28, 2021 01:21
.where(ClusterSubscription.user_id == context.user_id)
.where(Cluster.is_official_cluster)
.order_by(Node.id)
.limit(100000)
Copy link
Member Author

@darrenvong darrenvong Sep 28, 2021

Choose a reason for hiding this comment

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

I kinda copied this from threads.py where 100000 is an arbitrarily "large enough" (?) number/page size cap so it should be sufficient to include all communities the user is in one page, but not unbounded

Copy link
Member

Choose a reason for hiding this comment

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

Whenever we convert this into a subquery, the list of clusters will not be extracted from the database and this arbitrary limit will go away. So it is fine for now :)

@darrenvong darrenvong changed the title Web/feature/events from user communities Show events from user communities on dashboard Sep 28, 2021
kalvdans
kalvdans previously approved these changes Sep 28, 2021
Copy link
Member

@kalvdans kalvdans left a comment

Choose a reason for hiding this comment

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

Backend looks good! There's still some failing frontend CI tests though.

app/backend/src/tests/test_events.py Show resolved Hide resolved
app/proto/events.proto Show resolved Hide resolved
app/backend/src/tests/test_events.py Show resolved Hide resolved
@darrenvong
Copy link
Member Author

There's still some failing frontend CI tests though.

It's because of this - https://gitlab.com/gitlab-com/gl-infra/production/-/issues/5590 😠 apparently the adjacent web-next job built and the tests from that passed though so I think we might be ok overall!

Copy link
Member

@lucaslcode lucaslcode left a comment

Choose a reason for hiding this comment

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

One thing

app/web/src/features/dashboard/MyEvents.tsx Outdated Show resolved Hide resolved
@darrenvong darrenvong force-pushed the web/feature/events-from-user-communities branch from ee7edd0 to a6e2a91 Compare October 1, 2021 01:38
).not.toBeInTheDocument();

// Simulates scrolling horizontally to the end
mockIsIntersecting(screen.getByRole("progressbar"), true);
Copy link
Member

Choose a reason for hiding this comment

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

nice

Comment on lines +67 to +69
fetchNext={isBelowSm ? fetchNextPage : undefined}
hasMore={hasNextPage}
isFetching={isFetching}
Copy link
Member

Choose a reason for hiding this comment

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

Wow, I totally forgot I put this functionality in :D

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, made my job very easy 😉

@darrenvong darrenvong merged commit d04e9db into develop Oct 2, 2021
@darrenvong darrenvong deleted the web/feature/events-from-user-communities branch October 2, 2021 02:36
@aapeliv aapeliv added release notes: done Was mentioned in release notes and removed release notes: pending Add to stuff that should be included in release notes labels Oct 14, 2021
aapeliv pushed a commit that referenced this pull request Apr 20, 2024
…er-communities

Show events from user communities on dashboard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.topic backend This issue relates to the python backend 1.topic frontend release notes: done Was mentioned in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add events from my communities to the dashboard.
4 participants