[usage] More useful notification #13406

laushinka · 2022-09-28T11:16:17Z

Description

Shows:

The team/user attributed in the billing.
The link to the relevant billing page.

REACHED	80%

Related Issue(s)

Fixes #13166

How to test

Preview env

Join this team.
You should see the notification already.

Release Notes

NONE

Documentation

Werft options:

/werft with-local-preview
If enabled this will build install/preview
/werft with-preview
/werft with-payment
/werft with-integration-tests=all
Valid options are all, workspace, webapp, ide

werft-gitpod-dev-com · 2022-09-28T11:16:28Z

started the job as gitpod-build-lau-notifications-13166.7 because the annotations in the pull request description changed
(with .werft/ from main)

werft-gitpod-dev-com · 2022-09-28T11:16:30Z

started the job as gitpod-build-lau-notifications-13166.8 because the annotations in the pull request description changed
(with .werft/ from main)

laushinka · 2022-09-28T13:06:04Z

/werft run

👍 started the job as gitpod-build-lau-notifications-13166.10
(with .werft/ from main)

jldec · 2022-09-29T16:01:47Z

Let's change the first sentence to

Your team ‘[team-name]’ has reached its usage limit.

Thanks for that suggestion @laushinka.

jldec · 2022-09-29T16:04:44Z

And for the personal usage account.

You have reached your usage limit.

laushinka · 2022-09-29T18:38:18Z

@jldec Changes done 🚀

easyCZ · 2022-09-29T19:13:35Z

Would it make sense to include in the warning message that 80% has been reached? Right now it says almost reached, but doesn't specify how urgent or how close the user is.
See internal thread

laushinka · 2022-09-29T21:40:31Z

Would it make sense to include in the warning message that 80% has been reached? See internal thread

@easyCZ Thanks! I can see how the uncertainty of the word "almost" can create some panic. If we specify a number with a wording like "Your team has reached 80% of its usage limit", we would have to commit reflecting the actual percentage, won't we? For example today I see the banner, but then I keep racking up the usage without increasing the limit, tomorrow or the next it wouldn't be 80% anymore.

easyCZ · 2022-09-30T05:13:51Z

We could also include the actual percentage in the message. We already check for 0.8 * usage limit so we know how much it is.

easyCZ · 2022-09-30T07:40:54Z

components/dashboard/src/AppNotifications.tsx

+                    className="gp-link hover:text-gray-600"
+                    href={
+                        notifications.length > 1
+                            ? `${gitpodHostUrl}t/${notifications[notifications.length - 1]}/billing`


Is it possible to use the topNotification variable here? It would avoid us having to do the lookup.

Do we actually need the gitpodHostURL? I think we could make the URL relative which would avoid having to specify it here

Is it possible to use the topNotification variable here?

How do you mean? topNotification is the first sentence that includes the team name. The second entry in the array is the Manage billing link.

Ah, my bad. I misread the contents of those.

easyCZ · 2022-09-30T07:43:22Z

components/server/ee/src/workspace/gitpod-server-impl.ts

+                switch (limit.attributionId.kind) {
+                    case "user": {
+                        if (limit.reached) {
+                            result.unshift(`You have reached your usage limit.`);
+                        } else if (limit.almostReached) {
+                            result.unshift(`You have almost reached your usage limit.`);
+                        }
+                        break;
+                    }
+                    case "team": {
+                        teamOrUser = await this.teamDB.findTeamById(limit.attributionId.teamId);
+                        if (teamOrUser) {
+                            result.push(teamOrUser?.slug);
+                            if (limit.reached) {
+                                result.unshift(`Your team '${teamOrUser?.name}' has reached its usage limit.`);
+                            } else if (limit.almostReached) {
+                                result.unshift(`Your team '${teamOrUser?.name}' has almost reached its usage limit.`);
+                            }
+                        }
+                        break;
+                    }


The nesting here makes it quite hard to follow the main flow of the method. What if we refactored this block into a separate method and do something like

const usageNotification = getUsageNotificationForAttributionID(...) if usageNotification.length > 0 { ... }

Sorry @easyCZ I was refactoring this but forgot to hold and now it's merged!

laushinka · 2022-09-30T08:37:50Z

@jldec also prefers putting 80% so I will replace the almost sentences with "... has reached 80% of its usage limit.'

easyCZ · 2022-09-30T08:39:50Z

I think you should say 80% or more as it starts showing at 80%, but continues to show even above that

laushinka · 2022-09-30T08:40:35Z

I think you should say 80% or more as it starts showing at 80%, but continues to show even above that

Yes, nice suggestion!

jldec · 2022-09-30T08:46:18Z

@laushinka I just tested this in a preview env .
After adding a team and configuring a test payment method for UBP I was immediately shown this notification.

laushinka · 2022-09-30T08:57:57Z

@laushinka I just tested this in a preview env . After adding a team and configuring a test payment method for UBP I was immediately shown this notification.

Aha, thanks!

jldec

LGTM

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed size/XS labels Sep 28, 2022

roboquat added size/S and removed size/XS labels Sep 28, 2022

laushinka force-pushed the lau/notifications-13166 branch 2 times, most recently from 8c7d1ab to a9aedd8 Compare September 29, 2022 15:17

roboquat added size/M release-note-none and removed size/S do-not-merge/release-note-label-needed labels Sep 29, 2022

laushinka marked this pull request as ready for review September 29, 2022 15:21

laushinka requested a review from a team September 29, 2022 15:21

roboquat removed the do-not-merge/work-in-progress label Sep 29, 2022

github-actions bot added the team: webapp Issue belongs to the WebApp team label Sep 29, 2022

laushinka force-pushed the lau/notifications-13166 branch 2 times, most recently from 287581d to 33ffda2 Compare September 29, 2022 17:00

laushinka requested a review from jldec September 29, 2022 18:37

easyCZ reviewed Sep 30, 2022

View reviewed changes

laushinka force-pushed the lau/notifications-13166 branch from 33ffda2 to a3d9767 Compare September 30, 2022 09:40

[usage] More useful notification

6dd22ea

laushinka force-pushed the lau/notifications-13166 branch from a3d9767 to 6dd22ea Compare September 30, 2022 10:27

jldec approved these changes Sep 30, 2022

View reviewed changes

roboquat merged commit 51adca1 into main Sep 30, 2022

roboquat deleted the lau/notifications-13166 branch September 30, 2022 11:12

roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Oct 3, 2022

[usage] More useful notification #13406

[usage] More useful notification #13406

Uh oh!

Conversation

laushinka commented Sep 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue(s)

How to test

Release Notes

Documentation

Werft options:

Uh oh!

werft-gitpod-dev-com bot commented Sep 28, 2022

Uh oh!

werft-gitpod-dev-com bot commented Sep 28, 2022

Uh oh!

laushinka commented Sep 28, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jldec commented Sep 29, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jldec commented Sep 29, 2022

Uh oh!

laushinka commented Sep 29, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

easyCZ commented Sep 29, 2022

Uh oh!

laushinka commented Sep 29, 2022

Uh oh!

easyCZ commented Sep 30, 2022

Uh oh!

easyCZ Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

laushinka Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

easyCZ Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

laushinka Sep 30, 2022

Choose a reason for hiding this comment

Uh oh!

laushinka commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

easyCZ commented Sep 30, 2022

Uh oh!

laushinka commented Sep 30, 2022

Uh oh!

jldec commented Sep 30, 2022

Uh oh!

laushinka commented Sep 30, 2022

Uh oh!

jldec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laushinka commented Sep 28, 2022 •

edited

Loading

laushinka commented Sep 28, 2022 •

edited by werft-gitpod-dev-com bot

Loading

jldec commented Sep 29, 2022 •

edited by werft-gitpod-dev-com bot

Loading

laushinka commented Sep 29, 2022 •

edited by werft-gitpod-dev-com bot

Loading

laushinka commented Sep 30, 2022 •

edited

Loading