-
Notifications
You must be signed in to change notification settings - Fork 308
Repair paydays and add self checks (fixes #204). #1902
Conversation
where timestamp > ts_start | ||
and timestamp < ts_end | ||
and amount < 0 | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose that we start using repair.sql
instead of branch.sql
for these sorts of things. We're not changing the schema here, we're modifying data (similarly with #1883).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that is a technicality easily achieved. What about the main point of the PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the main point of the PR?
Sorry, I switched to #204 for that.
charge_volume = self.all(""" | ||
select * from ( | ||
select id, ts_start, charge_volume, ( | ||
select sum(amount+fee) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really want amount+fee
here? mark_charge_success
only increments by amount. Is this goofy due to ... that ticket that I can't find by searching for "amount fee" but in which you laid out the goofiness of the interaction between amount and fee?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check. But this is what fits most of the data that is already in the paydays table. The ticket you mentioned is #1118.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mark_charge_success is called from record_charge. The exchange table is updated by amout
where paydays table is updated by charge_amout
. These two values are sent to record_charge as params. The relation between the two is amount = charge_amount - fee
. I must say I don't really understand why we have this but I guess there was a reason (rounding?). Anyway, the code above is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ticket you mentioned is #1118.
Thanks, I was thinking of #1118 (comment) and #1118 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repair paydays and add self checks (fixes #204).
Adds self checks as suggested at #204.
Fixes to the
paydays
table are included in branch.sql.@whit537 Could you review please?