This is a refactor with a few incidental user-facing changes (eg not
showing a sometimes-incorrect total count in the UI).
---
Make `usePageSwitcherPagination` and `useShowMorePaginationUrl` support
storing filter and query params, not just pagination params, in the URL.
This is commonly desired behavior, and there are many ways we do it
across the codebase. This attempts to standardize how it's done. It does
not update all places this is done to standardize them yet.
Previously, you could use the `options: { useURL: true}` arg to
`usePageSwitcherPagination` and `useShowMorePaginationUrl`. This was not
good because it only updated the pagination URL querystring params and
not the filter params. Some places had a manual way to update the filter
params, but it was incorrect (reloading the page would not get you back
to the same view state) and had a lot of duplicated code. There was
actually no way to have everything (filters and pagination params)
updated in the URL all together, except using the deprecated
`<FilteredConnection>`.
Now, callers that want the URL to be updated with the connection state
(including pagination *and* filters) do:
```typescript
const connectionState = useUrlSearchParamsForConnectionState(filters)
const { ... } = usePageSwitcherPagination({ query: ..., state: connectionState}) // or useShowMorePaginationUrl
```
Callers that do not want the connection state to be reflected in the URL
can just not pass any `state:` value.
This PR also has some other refactors:
- remove `<ConnectionSummary first>` that was used in an erroneous
calculation. It was only used as a hack to determine the `totalCount` of
the connection. This is usually returned in the connection result itself
and that is the only value that should be used.
- remove `?visible=N` param from some connection pages, just use
`?first=N`. This was intended to make it so that if you reloaded or
navigated directly to a page that had a list, subsequently fetching the
next page would only get `pageSize` additional records (eg 20), not
`first` (which is however many records were already showing) for
batch-based navigation. This is not worth the additional complexity that
`?visible=` introduces, and is not clearly desirable even.
- 2 other misc. ones that do not affect user-facing behavior (see commit
messages)
## Test plan
Visit the site admin repositories and packages pages. Ensure all filter
options work and pagination works.
At a high level, we don't want to show annoying ads/upsells for Cody
that are not useful. And if Cody is disabled, we don't want to show
*any* links to Cody.
### Detailed desired behavior
- Dotcom
- Navbar
- Unauthed: "Cody" single link to /cody (marketing page)
- Authed: "Cody" dropdown with "Dashboard" (/cody/manage) and "Chat"
(/cody/chat)
- Routes
- /cody: always the marketing page
- /cody/manage: requires sign-in, shows Cody PLG subscription status for
the user (Free plan is auto-opted-into by default)
- /cody/chat: requires sign-in
- Enterprise with Cody enabled on instance
- Navbar
- Cody NOT enabled for current user: "Cody" single link to
/cody/dashboard
- Cody enabled for current user: "Cody" dropdown with "Dashboard"
(/cody/manage) and "Chat" (/cody/chat)
- Routes
- /cody: this link should not be present anywhere, but redirect to
/cody/dashboard
- /cody/manage: informational page, with editor/web links for
Cody-enabled users and a "contact admin to get access" message for
Cody-disabled users
- /cody/chat: chat for Cody-enabled users, redirect to /cody/manage for
Cody-disabled users
- Enterprise with Cody NOT enabled on instance (`"cody.enabled": false`
in site config)
- Navbar: no Cody link or dropdown
- Routes: all Cody routes 404
- All
- Do not show a Cody upsell on the /search page
This is an example of what we will KEEP for users on instances with Cody
enabled but who do not themselves yet have access to Cody. This is
useful because it informs users how to get access to Cody, and
presumably their site admin wants people to request it who want to use
it.
Fixes
https://linear.app/sourcegraph/issue/SRCH-529/hide-cody-ai-tab-and-cody-upsell-if-cody-is-not-enabled
### Unexpected code changes needed
This ended up being a much bigger change than I expected because I found
error-prone code that needed cleaning up:
- Improve how we determine if Cody is enabled in the frontend code.
Previously, we checked the license features in some places,
`cody.enabled` site config in others, and the user's current RBAC
permissions for Cody in yet others. The most error-prone was checking
the license features, since a license may entitle the instance to Cody
but the site admin may still choose to disable it. There were no places
in the frontend code where checking the license's entitlements was
actually correct, so I changed everything to checking either
`window.context.codyEnabledOnInstance` or
`window.context.codyEnabledForCurrentUser`.
- Did the same for `window.context.codeSearchEnabledOnInstance` for
symmetry.
- Removed "helper" functions that just checked 1 or 2 boolean values on
`window.context` related to this, in favor of accessing `window.context`
directly. Globals aren't great, and we should use React context or
something similar, but now that the JSContext has the right fields
(i.e., enabled instead of licensed), it's simpler and there is no need
for helper functions.
- Removed prop drilling of the `licenseFeatures` that was unnecessary
since these values are available in globals and were being set from
globals at some arbitrary point in the React component hierarchy anyway.
- Updated the GlobalNavbar test snapshots.
## Test plan
Run in 3 modes: (1) dotcom mode, (2) `"cody.enabled": false` in site
config, (3) normal `sg start`.
## Changelog
- When Cody is disabled in site config (with `"cody.enabled": false`),
all links and UI elements about Cody are hidden from all users.
Previously, when Cody was disabled, users would see some links informing
them about Cody.
* reapply "switch from jest to vitest for faster, simpler tests (https://github.com/sourcegraph/sourcegraph/pull/57886)"
This was reverted in https://github.com/sourcegraph/sourcegraph/pull/58116 due to an issue with the browser tests.
* include fetch-mock
* fix flakiness
* rm mock that did not work in experimentalVmThreads
* fix
* timeout
* fixup
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
When running vitests concurrently the target //client/browser:test fails
Revert "switch from jest to vitest for faster, simpler tests (#57886)"
This reverts commit ae5325e432.
Replaces our usage of jest with vitest. Also removes the babel transpiler. This simplifies our test configuration by a lot, makes tests run 10% faster, and makes further modernizations to our build/test stuff possible (such as using vite for frontend builds).
This removes some of the junit exporting for Buildkite, and the vitest bazel defs don't really cleanly implement bazel testing guidelines (like sharding). But vitest is only used for unit tests (all integration/e2e/regression tests have always run in mocha), so none of them are very slow anyway.
## Codemods for vitest imports
fastmod -e js,ts,tsx @jest/globals vitest client/ dev/release/
fastmod -e js,ts,tsx 'jest\.(\w+)\(' 'vi.$1(' client/ dev/release/
fastmod -e js,ts,tsx 'jest,' 'vi,' client/ dev/release/
fastmod -e js,ts,tsx 'jest }' 'vi }' client/ dev/release/
git diff --diff-filter=M --name-only | xargs pnpm exec prettier --write
