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: render Organization notifications on details page #11023

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Jan 10, 2024

This work was missed when implementing the new notification system.

Screenshot_2024-01-10_10-56-20

Related readthedocs/ext-theme#260

This work was missed when implementing the new notification system.

Related readthedocs/ext-theme#260
@agjohnson
Copy link
Contributor

Also, I'm guessing you may need to update the commercial templates with the missing logic.

@humitos
Copy link
Member Author

humitos commented Jan 11, 2024

Also, I'm guessing you may need to update the commercial templates with the missing logic.

I'm not sure to follow you here. I updated community templates for the old dashboard, which are used on commercial instance. In fact, the screenshot I uploaded in the description is from my commercial development instance. Am I missing something here?

@humitos humitos merged commit 74fa96c into main Jan 11, 2024
7 checks passed
@humitos humitos deleted the humitos/organization-notifications branch January 11, 2024 08:45
@agjohnson
Copy link
Contributor

@humitos Gotcha, it wasn't clear to me that screenshot was the updated notification.

I don't recall what we've overridden in corporate at this point, I haven't touched these templates for a while. Organization notifications seem good, but I would still look through these templates to ensure all the logic is replicated there. Especially for any templates that override community or do not inherit from community.

The base template doesn't inherit and seems to be missing the loops around notifications on corporate for example: https://github.com/readthedocs/readthedocs-corporate/blob/a7343911b82220c3d8c2d9348cd4b9e009868e32/readthedocsinc/corporate/templates/base.html#L61-L81

@humitos
Copy link
Member Author

humitos commented Jan 15, 2024

Cool! I opened https://github.com/readthedocs/readthedocs-corporate/issues/1709 to keep track of this 👍🏼

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.

2 participants