minder

minder copied to clipboard

### Describe the issue Our logs have sporadic uses of `projectID`, `project_id`, with a similar problem happening for entities and providers. We should decide on one and use that around...

JAORMX

The new Entity management framework added a bunch of SQL statements and the respective service layer. Code review showed that some of them are no longer necessary and some of...

blkt

Ensure a transaction is open in `ReplaceAllProperties` routine

3

[This routine](https://github.com/stacklok/minder/blob/44d1523d784f4684439c85f8863013416025204a/internal/entities/properties/service/service.go#L266-L275) must ensure that a transaction is open before performing the delete/insert sequence of statements.

blkt

it's unused

jhrozek

This is not used, and it also allows us to get rid of the `GetProperty` database call.

JAORMX

This provides us with some hardening that makes sure callers cannot even attempt to get an entity from a project that doesn't belong to them. Related to https://github.com/stacklok/minder/issues/4417

JAORMX

It's unused

jhrozek

This provides us with some hardening that makes sure callers cannot even attempt to get an entity from a project that doesn't belong to them.

JAORMX

Pass in the providerID to DeleteEntityByName

1

we're calling DeleteEntityByName to get rid of closed PRs. At that point we have the providerID but we're not passing it to the DB Delete function.

jhrozek

check if we need the property db wrappers

1

we have property db wrappers like GetAllPropertyValuesV1 or UpsertPropertyValueV1 but then we started using higher-order model structures like models.DbPropsToModel. We should converge on one or the other.

jhrozek