feat: Premium Page CTA updates and form handling - #28226
Conversation
…for frontend and back
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
untra
changed the title
untra
changed the title
2bb446a to
525e204
Compare
861e8d2 to
cd3f9f7
Compare
Documentation CheckUpdates Needed
Notes
Automated review via Coder Agents |
aslilac
left a comment
aslilac
left a comment
There was a problem hiding this comment.
this pr is waaaaaaaaay too big to review properly. it's at least three different changes rolled up into one. we target a pr size of +500 lines, with a max of +1000.
| ProxyHealthInterval: options.DeploymentValues.ProxyHealthStatusInterval.Value(), | ||
| DefaultQuietHoursSchedule: options.DeploymentValues.UserQuietHoursSchedule.DefaultSchedule.Value(), | ||
| ProvisionerDaemonPSK: options.DeploymentValues.Provisioner.DaemonPSK.Value(), | ||
| TrialLicenseRequester: trialer.NewLicenseRequester(trialer.LicenseRequestURL), |
There was a problem hiding this comment.
this should not be a separate object, we should extend the existing TrialGenerator to have whatever functionality is needed.
There was a problem hiding this comment.
I extended TrialGenerator and refactored the copypasta paths of license.
This approach and removing the trialgenerator option made writing golang tests a bit harder. I removed a few tests around trial license success generation because I wanted to avoid golang tests mocks, and I added a playwright e2e test to verify this continues to work.
https://github.com/coder/coder/actions/runs/32324209039/job/96292052395?pr=28226
Please let me know any other feedback! I try to always write tests to enforce correctness, but my golang-fu isn't the strongest , I defer to your wisdom 🙇
|
|
||
| // Keep in sync with cli/login.go. The values are forwarded to the Coder licensor, | ||
| // so changing them requires coordinating with the licensor service. | ||
| export const numberOfDevelopersOptions = [ |
There was a problem hiding this comment.
files that only contain consts are a huge code smell. who uses these consts? what functionality do they offer? are there multiple consumers? if they're really that important and useful why are there not any functions alongside them to help you utilize them properly?
There was a problem hiding this comment.
marketing wants to be able to have a say over copy, lots of little rich text coordinated strings and copy, and wants to be able to find and edit that copy.
With this premium upsell, there are a lot of small strings that need to be maintained and associated, as well as common links used in the SetupPage and LicensePage and the PremiumPage . It's rule of two going on three.
I did not know bulk consts are seen as a code smell in typescript. What defect does that smell suggest? Is there a better place to coordinate such strings or shared yup form validation?
There was a problem hiding this comment.
hm, I do feel a few of these do belong closer to the component (for example PREMIUM_TRIAL_UPSELL appears in exactly one component), and the Yup schema is definitely coupled to the trial form. I understand the reason behind wanting to keep marketing copy etc in one file for easy editing, but since marketing will likely be using agents, I wonder if there's a different way we can indicate or direct them to what they should change (comments?).
6f7323c to
099884b
Compare
…Premium branding on trial page Rework the trial signup page copy to lead with "Coder trial" language instead of "Premium": - headline now reads "Start an unlimited 30-day Coder trial" - subheadline focuses on Coder Agents usage and enterprise AI governance - page title/header changed from "Premium trial" to "Start a Coder trial" - pricing link text simplified to "Learn more" and points to the pricing comparison anchor - trial-active panel drops the "upgrade to Coder Premium" phrasing - database load acknowledgement references "Coder trial features" instead of "Premium features" - left nav item renamed from "Premium" to "Trial Upgrade"
…emium badge from externalauthsettingspage.
There was a problem hiding this comment.
nit, and for a future PR: wondering if we should use cva for this inside <Supergraphic>. if there are classes we supply again and again, that probably makes sense
jeremyruppel
left a comment
jeremyruppel
left a comment
There was a problem hiding this comment.
@untra the only comment I have left is perhaps breaking up site/src/modules/licenses/trialLicense.ts, but idk how far you want to take the cleanup, so I'm going to approve ahead of that. nice work!
…entiated, comment cleanup
…r/coder into feat/DEVEX-752-premium-page
all feedback has been addressed, aside from the PR size, but this is about as small as we can make it. we have code freeze tonight, so I'm dismissing this review so we can move forward
4 participants
?success=truequeryparam and confettisequenceDiagram autonumber actor Owner participant UI as UI<br/>TrialRequestForm + PremiumPage participant API as API<br/>api.ts + coderd middleware participant Coderd as Coderd<br/>postTrialLicense + trialer participant Licensor as Licensor<br/>v2-licensor + Postgres + pubsub + cache Note over UI: mutation.status = "idle" Owner->>UI: fills 10 fields, checks acknowledgement UI->>UI: Yup validate, strip "acknowledged", submit UI->>API: POST /api/v2/licenses/trial<br/>cookie + X-CSRF-TOKEN Note over UI: mutation.status = "pending" API->>API: verify CSRF, apiRateLimiter,<br/>apiKeyMiddleware sets actor API->>Coderd: dispatch Coderd->>Coderd: audit.InitRequest, authorize license:create,<br/>httpapi.Read validates, HasLicense is false Coderd->>Licensor: POST trial request<br/>deployment_id unspoofable, 10s deadline Licensor-->>Coderd: 200, raw signed JWT Coderd->>Coderd: ParseClaimsIgnoreNbf, uuid.Parse Coderd->>Licensor: InsertLicense, updateEntitlements,<br/>publish PubsubEventLicense, commit audit log Licensor-->>Coderd: database.License Coderd-->>API: 201 codersdk.License API-->>UI: 201 codersdk.License Note over UI: mutation.status = "success" UI->>Licensor: invalidate entitlements + licenses keys UI->>Owner: navigate /deployment/premium?success=true Note over Owner: confetti fires on the success param