GutoVeronezi commented on PR #8243:
URL: https://github.com/apache/cloudstack/pull/8243#issuecomment-2043303604
> > @winterhazel, you mentioned in the description that this PR also affects
quotaStatement and quotaBalance; however, I am not seeing changes in their
command classes. Could you check?
>
> The changes I mentioned were made in their workflow, not in their command
classes.
>
> Regarding `quotaStatement`, the following block has been removed from
`org.apache.cloudstack.quota.QuotaServiceImpl#getQuotaUsage` to allow providing
end dates in the future:
>
> ```diff
> - if (endDate.after(_respBldr.startOfNextDay())) {
> - throw new InvalidParameterValueException("Incorrect Date
Range. End date:" + endDate + " should not be in future. ");
> - }
> ```
>
> The same restriction has been removed for `quotaBalance` in
`org.apache.cloudstack.quota.QuotaServiceImpl#findQuotaBalanceVO`:
>
> ```diff
> - Date adjustedStartDate = computeAdjustedTime(startDate);
> - if (endDate.after(_respBldr.startOfNextDay())) {
> - throw new InvalidParameterValueException("Incorrect Date
Range. End date:" + endDate + " should not be in future. ");
> - } else if (startDate.before(endDate)) {
> - Date adjustedEndDate = computeAdjustedTime(endDate);
> + if (startDate.before(endDate)) {
> ```
>
> > Also, I checked the code and observed that both have an odd behavior:
they take the end date passed as parameters and calculate the next day to it,
to use it as the end date, instead of using what the caller passed, which means
it would not retrieve what the caller requested. It would be good to check this
behavior as well (Adjustment 2).
>
> We can check if this behavior makes sense and, if it does not, change how
these two APIs use the provided end date; but I think this change starts to get
out of the scope of this PR, and should be done in a separate one.
>
> > @winterhazel, you mentioned the API quotaTariffDelete in the PR's
description; however, I am not seeing changes in its command class. I also
checked the code and have not found a reason for this API being handled in this
PR. Was it a typo?
>
> As well as `quotaStatement` and `quotaBalance`, the mentioned change was
made in the workflow, not in the command class. In
`org.apache.cloudstack.api.response.QuotaResponseBuilderImpl#deleteQuotaTariff`,
I made it so that the actual removal date is persisted instead of a
manipulated one:
>
> ```diff
> - quotaTariff.setRemoved(_quotaService.computeAdjustedTime(new
Date()));
> + quotaTariff.setRemoved(new Date());
> ```
Thanks for the explanation, @winterhazel. It makes sense to handle/discuss
the `endDate` case in another PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
