Validation of repository_storages_weighted is spread across the class and concern, difficult to test
The following discussion from !170457 (merged) should be addressed:
-
@drew started a discussion: So the undercoverage on 705 seems to be legitimately never run, but it seems legitimately unreachable because of the
coerce_repository_storages_weighted
implementation:def coerce_repository_storages_weighted repository_storages_weighted.transform_values!(&:to_i) end
which is run before validation by ApplicationSetting.
This validation that lives in
ApplicationSettingImplementation
is like a... backup validation? That would run ifApplicationSetting
wasn't written the way it was? ButApplicationSettingImplementation
doesn't seem to have it's own spec.This is a kind of messy blend of responsibility shared between the model and the concern, and we should probably sort it out to make it easier to understand which code is responsible for what. I'm going to make a follow-up about this and apply pipeline:skip-undercoverage to let this refactor pass
👀