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

Notification about beta release #183

Closed
wants to merge 2 commits into from
Closed

Conversation

humitos
Copy link
Member

@humitos humitos commented Jun 26, 2023

I'd like to start promoting the new Beta dashboard. I think we are ready for it now. I've been using it during the last weeks and I haven't found big issues.

My idea here is to send a notification to all users. I wrote this code that we can use (after improving the copy):

from django.utils.translation import gettext_lazy as_
from readthedocs.notifications.notification import SiteNotification
from messages_extends.constants import SUCCESS_PERSISTENT

class BetaDashboardNotification(SiteNotification):

    success_message = _(
        'We are launching our new <strong>Beta dashboard</strong>. '
        '<a href="https://beta.readthedocs.org/">Give it a try</a> and send us feedback.'  # noqa
    )
    success_level = SUCCESS_PERSISTENT


for u in User.objects.filter(profile__banned=False).iterator():
    BetaDashboardNotification(user=u, success=True).send()

This notification will be shown to users on the old dashboard. They can dismiss the notification by clicking on the link or in the cross:

Screenshot_2023-06-26_09-49-48

Besides, when they arrive to the new beta dashboard they will see another notification saying this instance is in beta and we appreciate them sending back feedback to us. This notification cannot be dismissed on purpose; that's why I put it on the HTML template directly.

Screenshot_2023-06-26_09-57-06

@humitos humitos requested a review from a team as a code owner June 26, 2023 08:02
@humitos humitos requested a review from agjohnson June 26, 2023 08:02
@agjohnson
Copy link
Contributor

We talked about easing into notifications here, as we probably already wanted to gate a beta, and time frames in July will be tricky with time off.

Continuing to point users to the beta in issues is still a great idea. We'll pick up the pace on a beta when I'm back in July.

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.

We definitely want this sort of UI element on the beta dashboard, especially as we start getting more general users trying it out.

But, it's probably going to be annoying to have this be a static block that can't be dismissed. This block will be on every page -- form confirmations, logout, etc.

If this was a dismissable notification I think that would be enough. Perhaps emit this on first log in to the beta instance?

readthedocsext/theme/templates/base.html Outdated Show resolved Hide resolved
@humitos
Copy link
Member Author

humitos commented Feb 26, 2024

I opened this 8 months ago. I think we are more than ready to start promoting the beta templates. I would like to move forward with this idea, I don't see any blocker on the ext-theme side. What do you think?

@agjohnson
Copy link
Contributor

agjohnson commented Feb 26, 2024

The issue I noted above originally is still a problem. As this is implemented, the block is not dismissable and will be on every page. The notification system we have implemented is a better option for this. I'd prefer something that feels more purposeful here even I suppose.

As for time frame, I don't have any worries for ext-theme. My main concerns on time frame would be for commercial mostly, as that is much less tested and not public beta yet.

Show a small notification to users so they know they can expect some bugs or
things that are not working perfectly. Telling them to report these back to us.
@humitos humitos force-pushed the humitos/beta-notification branch from f1f98d9 to 7889185 Compare February 27, 2024 11:50
@humitos
Copy link
Member Author

humitos commented Feb 27, 2024

Dismissing a notification is not implemented on the new dashboard yet, and it will also require #259 to do it properly. I'm fine adding this as a static block for now and improve it in the next iteration. My main goal here is to get more people using the new dashboard --it's objectively better than the old one, but nobody is taking advantage of it.

If we don't want to go this way, I'm also fine not adding this notification in the ext-theme templates; but I really want to add a notification in the old dashboard pointing people to the new one. We can use the new notification system there to attach a one-time and dismissable notification to all the users.

@agjohnson
Copy link
Contributor

Yeah, I think you are talking more about adding a link from the legacy dashboard here. A notification here is just an exit path back if things go wrong. We do need this and it should be easy to find.

I don't feel a static block at the top of all pages is the best approach though. It interferes with the design on all pages -- forms, 404 pages, etc. There are lots of downstream places this doesn't fit.

I would suggest a more limited approach in both cases, like only on the main dashboard template. We already discussed starting with the build detail page, which is a step before this all.

@humitos
Copy link
Member Author

humitos commented Feb 27, 2024

OK. I'm closing this PR since we are not going to move in the static block direction --which is what this PR was about.

I would suggest a more limited approach in both cases, like only on the main dashboard template

We can start there, then 👍🏼 . This can be achieved by creating a one-time-dismissable notification as I mentioned before.

@humitos humitos closed this Feb 27, 2024
humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Feb 27, 2024
We can add the notifications to all the users by running the following code:

```python
from readthedocs.core.notifications import MESSAGE_BETA_DASHBOARD_AVAILABLE

for user in User.objects.all():
    Notification.objects.add(
      message_id=MESSAGE_BETA_DASHBOARD_AVAILABLE,
      attached_to=user,
      dismissable=True,
    )
```

This will show a one-time dismissable notification to these users.
In the future (weeks, months) if we can show them another notification like this
one as a reminder, we can run the same code again.

Related readthedocs/ext-theme#183
humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Feb 27, 2024
We can add the notifications to all the users by running the following code:

```python
from readthedocs.core.notifications import MESSAGE_BETA_DASHBOARD_AVAILABLE

for user in User.objects.all():
    Notification.objects.add(
      message_id=MESSAGE_BETA_DASHBOARD_AVAILABLE,
      attached_to=user,
      dismissable=True,
    )
```

This will show a one-time dismissable notification to these users.
In the future (weeks, months) if we can show them another notification like this
one as a reminder, we can run the same code again.

Related readthedocs/ext-theme#183
humitos added a commit to readthedocs/readthedocs.org that referenced this pull request Feb 29, 2024
* New dashboard: notification to point users there

We can add the notifications to all the users by running the following code:

```python
from readthedocs.core.notifications import MESSAGE_BETA_DASHBOARD_AVAILABLE

for user in User.objects.all():
    Notification.objects.add(
      message_id=MESSAGE_BETA_DASHBOARD_AVAILABLE,
      attached_to=user,
      dismissable=True,
    )
```

This will show a one-time dismissable notification to these users.
In the future (weeks, months) if we can show them another notification like this
one as a reminder, we can run the same code again.

Related readthedocs/ext-theme#183

* Update readthedocs/core/notifications.py

Co-authored-by: Anthony <[email protected]>

* Change the content of the notification based on old/new dashboard

* Test fixed

* Update readthedocs/core/notifications.py

Co-authored-by: Anthony <[email protected]>

---------

Co-authored-by: Anthony <[email protected]>
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