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.

Sign up for GitHub

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

Jump to bottom

ui: add release notes signup to admin UI #43912

Closed
piyush-singh opened this issue Jan 13, 2020 · 5 comments · Fixed by #45143, #44856, #44821 or #44744
Closed

ui: add release notes signup to admin UI #43912

piyush-singh opened this issue Jan 13, 2020 · 5 comments · Fixed by #45143, #44856, #44821 or #44744
Assignees
@koorosh @piyush-singh
Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category.

Comments

@piyush-singh
Copy link

We should add the ability to sign up for CRDB release notes and updates directly to the admin UI. cc @Annebirzin - let's discuss how this will work.
@piyush-singh piyush-singh added the A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category. label Jan 13, 2020
@piyush-singh piyush-singh self-assigned this Jan 13, 2020
@Annebirzin
Copy link

Designs are here: https://zpl.io/2pXPNMJ

@piyush-singh
Copy link
Author

Follow up to the discussion @dhartunian and I had yesterday:

We do not want to add the heavyweight javascript libraries necessary to enable Hubspot forms into our installed product. Further, we can't simply add calls into an API as our that would require setting up vault or something similar to handle secrets like our API keys. This is further complicated by the source availability of crbd.

Instead, we'd like to use a pattern that David had used successfully in CockroachCloud. We will send emails to Hubspot via Segment, our analytics tracking service. You can use segment to fire an identify event that will be processed by their automation and inserted into Hubspot. This will give us a way to test this endpoint as well, as we can see the events sent to segment in real time while testing the new email capture.

You can set up the new segment source that I created specifically for the email signups by using this key: 72EEC0nqQKfoLWq0ZcGoTkJFIG9G9SII.

NOTE: there is an existing analytics client, and we should leave that as is. We would like to set up a new client using the key above ONLY for the email signup field.

@piyush-singh
Copy link
Author

Andrii's note from sprint planning today: should we extract the key for segment and instead fill that in during our build process?

@koorosh koorosh self-assigned this Feb 6, 2020
This was referenced Feb 7, 2020
Merged
Merged
@koorosh koorosh mentioned this issue Feb 18, 2020
Merged
2 tasks
@koorosh koorosh mentioned this issue Feb 21, 2020
Merged
@craig craig bot closed this as completed in bef2eb2 Feb 25, 2020
@koorosh
Copy link
Collaborator

koorosh commented Feb 25, 2020

Reopened. Waiting for two more PRs to be merged: #44856 , #45143

@koorosh koorosh reopened this Feb 25, 2020
@dhartunian
Copy link
Collaborator

@koorosh can you also add the following traits to the Segment event?

release_notes_sign_up_from_admin_ui: "true"
product_updates: "true"

@craig craig bot closed this as completed in 7b4af8c Feb 25, 2020
@dhartunian dhartunian reopened this Feb 26, 2020
craig bot pushed a commit that referenced this issue Feb 28, 2020
@koorosh @yuzefovich
@irfansharif @danhhz @pbardea
Merge #45143 #45426 #45455 #45535
38a7b75 
45143: ui: Release notes signup r=dhartunian a=koorosh

Resolves: #43912
Depends on: #44744, #44821, #44856 

- [x] WIP. Current branch has two branches merged which haven't been approved/merged in master branch yet.

- [x] rebase branch to remove merge commits from branches other than master.

Add Release notes signup form to Cluster Overview page
right after page title.
Release Notes signup view is created in `src/views/dashboard`
directory because it will be the main page where we display
this view. And Cluster Overview page is a temporary place
while Dashboard view doesn't exist.

These changes integrate three main parts: ReleaseNotesForm,
AlertNotification component and custom analytics saga.



45426: coldata: minor tweak of flat bytes r=yuzefovich a=yuzefovich

This commit changes `maybeBackfillOffsets` to update `maxSetIndex`
accordingly (this might be a minor performance improvement). In a sense,
when we're backfilling offsets, we *are* setting indices to point to
empty `[]byte` slices. Also, the logic for `Set` method is slightly
refactored.

Release note: None

45455: clusterversion: significantly rework cluster version handling r=tbg a=irfansharif

We split off ClusterVersion out of pkg/settings/cluster into a dedicated
pkg/clusterversion. This is to pave the way for #39182 where we
introduce long running version migration infrastructure.

We then introduce clusterversion.Handle as a read only view to the
active cluster version and this binary's version details. We similarly
fold in the actual global cluster version setting into
pkg/clusterversion, and enforce all external usage to go through
clusterversion.Handle. We can also hide away external usage of the baked
in cluster.Binary{,MinimumSupported}Version constants. Instead we have
the implementation of clusterversion.Handle allow for tests to override
binary and minimum supported versions as needed.

Along the way we clean up Settings.Initialized, as it is not really
used. We document all the various "versions" in play ("binary version",
    "minimum supported version", "active version") and change usage of what
was previously "serverVersion" to simply be "binaryVersion", because
that's what it is. We also clean up the Settings constructors into
Test/Non-Test types that differ in cluster version setting
initialization behaviour.

---

For reviewers: It's probably easiest to start with what 
pkg/settings/cluster/cluster_settings.go looks like, then following into
pkg/clusterversion/cluster_version.go and then pkg/clusterversion/setting.go.

---

I still don't like the following detail about our pkg structure:

- pkg/settings/cluster depends on it's "parent" pkg, pkg/settings
- pkg/settings/cluster depends pkg/clusterversion, which in turn depends
on pkg/settings

Naively, pkg/settings/cluster should be a top level package, but I'm not
making that change in this PR. For now I've renamed the settings.go file
to cluster_settings.go.

Release note: None


45535: Revert "storage,libroach: update MVCCIncrementalIterator to look at every updated key" r=pbardea a=pbardea

Reverts #45163

To stop the errors we're seeing on #45524. Will investigate further once it's off master.

Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Daniel Harrison <daniel.harrison@gmail.com>
Co-authored-by: Paul Bardea <pbardea@gmail.com>
@craig craig bot closed this as completed in c778c12 Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@koorosh koorosh

@piyush-singh piyush-singh

Labels
A-webui-general Issues on the DB Console that span multiple areas or don't have another clear category.
Projects
None yet
Milestone
No milestone
4 participants
@dhartunian @koorosh @piyush-singh @Annebirzin