Move more provider access methods to store #2911
Conversation
dmjb
force-pushed
the
provider-store-more-methods
branch
2 times, most recently
from
42fbc17 to
f258eb5
Compare
| Name: dbRepo.Provider, | ||
| Projects: ph, | ||
| }) | ||
| prov, err := s.providerStore.GetByName(ctx, dbRepo.ProjectID, dbRepo.Provider) |
There was a problem hiding this comment.
Genuine question: The old code was calling GetParentProjects and then getting the provider by name in parent. The new code goes to GetByName, which calls findProjects which calls getProjectHierarchy. Are those equivalent?
There was a problem hiding this comment.
getProjectHierarchy also calls GetParentProjects. In fact, I may get rid of the separate getProjectHierarchy function since it's only called in one places (originally in this branch it was called in a second place too)
There was a problem hiding this comment.
That being said - one important difference is that the GetByName will check to see if there are any duplicate results and return an error if there is ambiguity about being what is returned, whereas the code I have replaced will not (see the implementation of GetByName).
@eleftherias - since I think you wrote this code: is there any possibility that the original query could match more than one provider?
There was a problem hiding this comment.
It is possible, but only in a strange corner case where someone renamed their GitHub organization to have the same name as a GitHub organization that they previously installed Minder on. We generally don't handle renames yet, so I think we can ignore that case for now.
Relates to #2845 Migrate more queries for providers to ProviderStore. This will make it easier to refactor away direct access to the provider table in subequent PRs.
dmjb
force-pushed
the
provider-store-more-methods
branch
from
e72b3e9 to
ee5a53e
Compare
Relates to #2845
Migrate more queries for providers to ProviderStore. This will make it easier to refactor away direct access to the provider table in subequent PRs.
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: