Skip to content

Conversation

filiptronicek
Copy link
Member

@filiptronicek filiptronicek commented Aug 26, 2024

Description

Make the experience of using a large amount of projects / repository configurations more delightful.

  • Removes the project context previously used by feature flags on the old /project paths
  • Gets rid of useListAllProjectsQuery, which was used in ways it shouldn't have (like getting the current project)

Related Issue(s)

Fixes ENT-717

How to test

First, join my org so you have some repos imported

Try starting a workspace:

  1. By selecting an SCM repo
  2. By selecting an imported repo
  3. By directly going to the page with an imported repo's URL
  • /werft with-preview

organizationId: org.id,
excludeConfigurations,
pagination: {
pageSize: 100,
Copy link
Member Author

Choose a reason for hiding this comment

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

We now query at most 100 projects (SCM repos not included) for the initial load in repo selectors. Pagination is not implemented, but also not necessary yet

@roboquat roboquat added size/XL and removed size/L labels Aug 27, 2024
Copy link
Member Author

Choose a reason for hiding this comment

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

The unified repo search has been changed to:

  • in addition to the SCM repo search, searching for any imported repository with the search term
  • also in addition to the above, we search for the currently selected imported repo either by ID or URL to resolve anything you might already have in there

return repo.configurationId === selectedConfigurationId;
}

if (repo.url.endsWith(".git")) {
Copy link
Member

Choose a reason for hiding this comment

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

🫧 We have similar code in the backend as well...

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this piece is duplicated in so many places. We should try to have this somewhere more centralized.

"tailwind-underline-utils": "^1.1.2",
"tailwindcss-filters": "^3.0.0",
"typescript": "^5.4.5",
"typescript": "^5.5.4",
Copy link
Member Author

Choose a reason for hiding this comment

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

doing this because I was using a feature introduced in 5.5

Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, which feature?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the Inferred Type Predicates, which lets us do filter with !== undefined without having to use something like repo is SuggestedRepository, basically typescript being smart about the fact that when you check something isn't undefined, its type correctly changes.

@mustard-mh
Copy link
Contributor

Workspaces worked well

.orderBy("project.creationTime", "DESC")
.andWhere("project.markedDeleted = false");

if (limit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit (not-blocking: Do you think we need a default value and max value (where we use it)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good to impose a limit here. Added that in 1c5a88d (#20151)

const suggestedQuery = useSuggestedRepositories({ excludeConfigurations });
// 1st data source: suggested SCM repos + up to 100 imported repos.
// todo(ft): look into deduplicating and merging these on the server
const suggestedQuery = useSuggestedRepositories({ excludeConfigurations: true });
Copy link
Contributor

@mustard-mh mustard-mh Aug 29, 2024

Choose a reason for hiding this comment

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

It's weired to do this change, I image that suggestedRespositories has some logic inside like base on users' activity recommend them with different priority of items.

In current implement, we will filter out configurations and fetch + append again only via name searching.

Just feel it's weired, not blocking

Copy link
Contributor

Choose a reason for hiding this comment

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

It leads a condition: I use bitbucket.gitpod-dev.com/scm/tes/ten-k-1304 to start a workspace, and back to dashboard, the first listed context url in new workspace page is not 1304

Copy link
Member Author

Choose a reason for hiding this comment

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

That's fair, we should not make this reorder all projects all of a sudden. Reverting this change

Copy link
Member Author

Choose a reason for hiding this comment

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

@mustard-mh
Copy link
Contributor

mustard-mh commented Aug 29, 2024

Workspaces worked well

@filiptronicek do you expect the third one tes-2krepos1000-vyclwfugewd could choose a configuration to use? I found it's not using a configuration in ListWorkspace

            "id": "tes-2krepos1000-vyclwfugewd",
            "metadata": {
                "ownerId": "159f9603-0886-4fe5-9467-8b1ae10c6955",
                "organizationId": "0b162669-fd42-46eb-891d-1fd24c16b626",
                "name": "TES/2k-repos-1000 - ",
                "originalContextUrl": "https://bitbucket.gitpod-dev.com/projects/TES/repos/2k-repos-1000"
            }

update: https://bitbucket.gitpod-dev.com/projects/TES/repos/2k-repos-1000 is not a configuration

Use a really imported configuration works well
image

repositories: repos.map((r) => this.apiConverter.toSuggestedRepository(r)),
pagination: new PaginationResponse({
nextToken: "",
total: repos.length,
Copy link
Member Author

Choose a reason for hiding this comment

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

we no longer know

@roboquat roboquat merged commit a1ec400 into main Aug 29, 2024
14 of 15 checks passed
@roboquat roboquat deleted the ft/improve-many-project-handling branch August 29, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants