Remove controller soft delete #1229
Conversation
internal/db/model.go
Outdated
| } | ||
| var models []dbmodel.Model | ||
| db := d.DB.WithContext(ctx) | ||
| err := db.Where("controller_id = ?", controllerID).Find(&models).Error |
There was a problem hiding this comment.
Prefer this approach, makes more sense to me. Really wish our domain objects had logic like this inside though. for example controller.LoadModels() - I guess another argument to refactor our whole "Database" vs "Models" thing going on here
There was a problem hiding this comment.
if we are going this way.. then what's the point in using gorm? should we simply abandon gorm in favor of sqlair?
There was a problem hiding this comment.
I mean the with gorm I don't need to write
fmt.Sprintf("SELECT * FROM models WHERE controller_id = %s",...)
Which is very helpful. And I'm sure we get other benefits that I can't think up right now. All this change means is that we don't get all the models every time we get a controller. We could just as easily remove our entire DB package and do the gorm queries in internal/jimm when we need them. So db.Controllers.Preload("Models").
There was a problem hiding this comment.
Changed it to more gorm appropriate syntax
There was a problem hiding this comment.
No you just use the gorm model and put its domain methods on the model itself
internal/db/model.go
Outdated
| } | ||
| var models []dbmodel.Model | ||
| db := d.DB.WithContext(ctx) | ||
| err := db.Where("controller_id = ?", controllerID).Find(&models).Error |
There was a problem hiding this comment.
if we are going this way.. then what's the point in using gorm? should we simply abandon gorm in favor of sqlair?
kian99
force-pushed
the
CSS-8891-remove-controller-soft-delete
branch
from
4f1e611 to
a80cee3
Compare
Description
This PR allows a controller to be removed from JIMM and then later re-added. This is not currently possible because removing a controller results in a soft-delete which is not handled properly when adding the controller again. To resolve this I've simply removed soft-deletes.
Another issue that cropped up because of this is that the function to remove a controller was also deleting all models associated with that controller first. This functions was broken because the
Modelsproperty on the controller was never populated when fetching a controller. This could be fixed by adding a.Preload("Models")on theGetControllerfunction but would be very unnecessary as the only place we use this reverse-relationship from a Controller->Models is when we remove a controller.Instead I've opted to introduce a new
GetModelByControllerfunction to do this action.Fixes CSS-8891
Engineering checklist
Check only items that apply