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

site admin: add link to Cody Analytics#60371

Merged
taras-yemets merged 6 commits into
mainfrom
ty/link-to-cody-analytics
Feb 9, 2024
Merged

site admin: add link to Cody Analytics#60371
taras-yemets merged 6 commits into
mainfrom
ty/link-to-cody-analytics

Conversation

@taras-yemets

@taras-yemets taras-yemets commented Feb 9, 2024

Copy link
Copy Markdown
Contributor

Adds a link to Cody Analytics service.
Figma

Screenshot 2024-02-09 at 18 45 33

Test plan

  • CI
  • Tested manually

@cla-bot cla-bot Bot added the cla-signed label Feb 9, 2024
@taras-yemets taras-yemets requested review from a team and rrhyne February 9, 2024 15:51
@taras-yemets taras-yemets added backport 5.3 backport/improvement Final touching of existing features labels Feb 9, 2024

@chwarwick chwarwick left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users will need to request access right? It's probably worth a note about how to do that.

@taras-yemets taras-yemets enabled auto-merge (squash) February 9, 2024 16:52
@taras-yemets taras-yemets merged commit d99889d into main Feb 9, 2024
@taras-yemets taras-yemets deleted the ty/link-to-cody-analytics branch February 9, 2024 17:08
sourcegraph-release-bot pushed a commit that referenced this pull request Feb 9, 2024
},
{
path: '/analytics/cody',
render: () => <AnalyticsCodyPage />,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @taras-yemets , I know it's a bit late but we should be gating this path in the side-admin page.

This should only show up when the license has Cody enabled. I realize you have a backport PR and this is merged but do you think you can push a fix for this to main and another to the backport branch

CleanShot 2024-02-12 at 01 43 09@2x

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, @BolajiOlajide! Thank you!
I ensured the page is not accessible via sidebar UI (https://github.com/sourcegraph/sourcegraph/pull/60371/commits/c71ffac46155b72378294bf113af234eb189322a), but missed to do the same on the route level.
Fixed in https://github.com/sourcegraph/sourcegraph/pull/60396.
I'll update the backport PR when the one to main is merged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @taras-yemets

keegancsmith pushed a commit that referenced this pull request Feb 12, 2024
* site admin: add link to Cody Analytics (#60371)

(cherry picked from commit d99889d)

* Render Cody analytics site admin route if Cody is enabled in license (#60396)

---------

Co-authored-by: Taras Yemets <yemets.taras@gmail.com>
@rrhyne

rrhyne commented Feb 12, 2024

Copy link
Copy Markdown

@taras-yemets here is the original screen I designed for this:

https://www.figma.com/file/pMaNBzIz3to8ujUsRX4f7I/Cody-analytics?type=design&node-id=8-32&mode=design&t=IhCdIeficqxYo6X2-4

I've replaced the image with a current snapshot of the design. Can we get that included so this page is a little less bare?

Also, we need a way to link back to the instance on the analytics site. The analytics service seems to know about the instance as it reads sourcegraph.sourcegraph.com in the top right, but it doesn't link to the instance, and doesn't contain anything when there is once instance so it appears broken.

image

@taras-yemets

Copy link
Copy Markdown
Contributor Author

Thanks for your feedback, @rrhyne!

I created a separate issue to address it, let's continue the discussion there.

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

Labels

backport/improvement Final touching of existing features backport 5.3 cla-signed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants