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

Notifications: show attached to User, Project and Organization #261

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 11, 2024

Render notifications attached to these objects.
This is the first pass of this work and there are some things we need to make decisions and improve.

  • Where (what pages) these notifications should render?
  • How "global" they should be considered?

Note we are rendering these notifications in the template for now, but in the future they will be rendered using the APIv3: #259

Screenshots

User

Screenshot_2024-01-11_11-01-33

Project

Screenshot_2024-01-11_11-01-40

Organization

Screenshot_2024-01-11_11-01-16

Closes #260

Render notifications attached to these objects.
This is the first pass of this work and there are some things we need to make
decisions and improve.

- Where (what pages) these notifications should render?
- How "global" they should be considered?

Note we are rendering these notifications in the template for now,
but in the future they will be rendered using the APIv3: #259

Closes #260
@humitos humitos requested a review from a team as a code owner January 11, 2024 10:05
@humitos humitos requested a review from agjohnson January 11, 2024 10:05
@humitos humitos force-pushed the humitos/notifications-user-project-organization branch from bbfd72c to 3e31aa6 Compare January 11, 2024 10:06
@humitos
Copy link
Member Author

humitos commented Jan 11, 2024

I'm showing all the notifications attached to Project and Organization. However, the endpoint we are creating will only show these notifications to admins/owners. @agjohnson should I do the same here by returning a more specific queryset via the get_context() in the view?

Copy link
Contributor

@agjohnson agjohnson left a comment

Choose a reason for hiding this comment

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

I'd say we should keep all of the notifications in the same location on the page for now, except for the special location in the build detail page. It will be a little more confusing if there are user and project notifications, as these would display at two different points in the page.

I would put the project/organization notifications at the top of the content page, above the project/organization header. I could see us revisiting this later though.

@humitos
Copy link
Member Author

humitos commented Jan 16, 2024

I would put the project/organization notifications at the top of the content page, above the project/organization header.

Done in 84073ef

It looks like this now:

Screenshot_2024-01-16_09-36-11

@humitos humitos requested a review from agjohnson January 16, 2024 08:38
@humitos humitos requested a review from agjohnson January 17, 2024 09:44
@humitos humitos merged commit df4192b into main Jan 18, 2024
3 of 4 checks passed
@humitos humitos deleted the humitos/notifications-user-project-organization branch January 18, 2024 12:41
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.

Add missing notification template blocks for user, project, organization notifications
2 participants