TSPS-405 Add quota units to pipeline quotas #205
Conversation
| @@ -51,15 +51,17 @@ public ResponseEntity<ApiQuotaWithDetails> getQuotaForPipeline(String pipelineNa | |||
| UserQuota userQuota = | |||
| quotasService.getOrCreateQuotaForUserAndPipeline( | |||
| user.getSubjectId(), validatedPipelineName); | |||
| String quotaUnits = quotasService.getPipelineQuota(validatedPipelineName).getQuotaUnits(); | |||
There was a problem hiding this comment.
could alternatively make a dedicated method just to retrieve the quota units from the db
There was a problem hiding this comment.
yea i like the idea of having a method for that. no need to grab extra info (even if it is little)
| @@ -51,15 +51,17 @@ public ResponseEntity<ApiQuotaWithDetails> getQuotaForPipeline(String pipelineNa | |||
| UserQuota userQuota = | |||
| quotasService.getOrCreateQuotaForUserAndPipeline( | |||
| user.getSubjectId(), validatedPipelineName); | |||
| String quotaUnits = quotasService.getPipelineQuota(validatedPipelineName).getQuotaUnits(); | |||
There was a problem hiding this comment.
yea i like the idea of having a method for that. no need to grab extra info (even if it is little)
| @@ -28,9 +28,14 @@ public class PipelineQuota { | |||
| @Column(name = "min_quota_consumed") | |||
| private int minQuotaConsumed; | |||
|
|
|||
| public PipelineQuota(PipelinesEnum pipelineName, int defaultQuota, int minQuotaConsumed) { | |||
| @Column(name = "quota_units") | |||
| private String quotaUnits; | |||
There was a problem hiding this comment.
because the data from this column is gonna be added through our db migrations only, its probably worth setting this up as an enum like pipeline_name above. It just gives us the extra security (with the related unit test) that we are entering things we expect into the database.
| columns: | ||
| - column: | ||
| name: quota_units | ||
| value: 'samples' |
There was a problem hiding this comment.
should we update the e2e to test for this too? prob not necessary
There was a problem hiding this comment.
we don't currently check all the fields returned from the quota endpoint... i could make the change to ask for all of them, but currently we only check the value of response['quotaConsumed']
|
| @Converter(autoApply = true) | ||
| public class QuotaUnitsConverter implements AttributeConverter<QuotaUnitsEnum, String> { | ||
| @Override | ||
| public String convertToDatabaseColumn(QuotaUnitsEnum quotaUnitsEnum) { |
There was a problem hiding this comment.
this is the one place that's not being tested. since we never actually use code to set this value in the db (we use liquibase), i'm ok with that, but i can construct a test if that would be better (we don't have explicit tests of these kinds of classes currently)
There was a problem hiding this comment.
yea i think thats more than fine to not test this. We could prob even throw like a "not implemented error dont call me" here and it wouldnt be an issue. that could be the solution :P
Description
Each pipeline should have its quota unit defined. This PR adds a
quota_unitsfield to thepipeline_quotastable in the teaspoons database; the string value ofQuotaUnitsEnumis meant to be the plural version of the unit, e.g. "samples" rather than "sample". We include this field in theQuotaWithDetailsAPI response that users receive when they query their quota status for a pipeline.Jira Ticket
https://broadworkbench.atlassian.net/browse/TSPS-405
Checklist (provide links to changes)