Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Use temporary settings to track configurable "time saved" analytics fields#42785

Merged
adeola-ak merged 1 commit into
aa/display-kpisfrom
kr/display-kpis-temp-settings
Oct 11, 2022
Merged

Use temporary settings to track configurable "time saved" analytics fields#42785
adeola-ak merged 1 commit into
aa/display-kpisfrom
kr/display-kpis-temp-settings

Conversation

@courier-new

Copy link
Copy Markdown
Contributor
Screen.Recording.2022-10-10.at.5.18.34.PM.mov

Test plan

Tested locally. Observed updated temporary settings in the database.

@courier-new courier-new requested a review from adeola-ak October 11, 2022 00:19
@courier-new courier-new self-assigned this Oct 11, 2022
@cla-bot cla-bot Bot added the cla-signed label Oct 11, 2022
@courier-new courier-new added site-admin Site admin experience batch-changes Issues related to Batch Changes labels Oct 11, 2022
@sourcegraph-bot

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff a104ee6...bf91e92.

Notify File(s)
@eseliger client/web/src/enterprise/batches/list/BatchChangeStatsBar.tsx

@adeola-ak adeola-ak merged commit a62213c into aa/display-kpis Oct 11, 2022
@adeola-ak adeola-ak deleted the kr/display-kpis-temp-settings branch October 11, 2022 00:54
@eseliger

Copy link
Copy Markdown
Member

I think temporary settings are per user, correct? Since only the site admin can access the page where it's set, how do regular users get any useful number on the BC list page?

Maybe a design question more than a code question, actually.

adeola-ak added a commit that referenced this pull request Oct 11, 2022
* set up backend with new changesets api

* display data

* add changeset status cells

* fix margin and go linting

* add tooltip

* missed migration?

* remove important

* lower case c

* remove space

* loading and error state

* remove label as its optional

* replace changeset connection with global stats

* save time save in local storage remove label

* add tests

* uncomment tests

* go fix

* fix stitched migration

* remove space from s.mig

* fix error and loading

* fix error handle

* error handling

* case for no data

* remove error import

* add story

* fix BatchChangeListPage broke story

* revert label

* add constant per item saved key variable

* Changesets merged

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

* Hours saved

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

* closed

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

* open

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

* feedback

* resolve comments

* Update cmd/frontend/graphqlbackend/batches.graphql

Co-authored-by: Erik Seliger <erikseliger@me.com>

* fixes

* add test

* uncomment tests

* Use temporary settings to track configurable "time saved" analytics fields (#42785)

* fix changeset cells

* remove null state

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>
Co-authored-by: Erik Seliger <erikseliger@me.com>
@courier-new

Copy link
Copy Markdown
Contributor Author

It's true, I feel like the calculation value part of this was under-spec'd in the ticket because we all assumed this was already saved somewhere. 😂 But hey, saving for admins is still better than not saving at all, right? Regular users will just get the default 15 minutes. Maybe we can file a follow-up ticket about saving this site-wide somehow.

@eseliger

Copy link
Copy Markdown
Member

Maybe we can file a follow-up ticket about saving this site-wide somehow.

I think that would be great :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

batch-changes Issues related to Batch Changes cla-signed site-admin Site admin experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants