-
Notifications
You must be signed in to change notification settings - Fork 1
Changes #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes #3
Conversation
… reasons to pay for stream (partial payment EventParticipation), so first select latest row with reason LeftPaidStream and then select all rows with reason != LeftPaidStream and with insertedTime more than prev row selected. (cherry picked from commit 8e73f427f2bc96fee88b4690a62302aa2a1f330c)
(cherry picked from commit 867d93f39e63ffbf903cf93227165fca4f0841c2)
classes/Assets/Credits.php
Outdated
| } | ||
|
|
||
| // as $joined_assets_credits ordered by insertedTime asc than the last row is the latest | ||
| $data->insertedTime = $joined_assets_credit->insertedTime; |
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.
It's not good to reference $joined_assets_credit which is a local variable in a loop, after loop ends. Yes this works in PHP and happens to leave it pointing to the last one, but are you sure this is what you want?
why are you making $data an StdClass() object? and then return it instead of an actual Assets_Credits row? what are you trying to accomplish here? it seems wrong. At least make it an Assets_Credits row, even though it's synthetic, so its type will match.
I guess you are trying to handle a result where multiple transfers happened after user left stream the previous time? Instead of one transfer?
I would say just change the function signature to return an array of Assets_Credits rows, and change all the callers of this method to traverse the array and do what they want (e.g. sum the amounts in all the rows).
classes/Assets/Credits.php
Outdated
| 'fromStreamName' => $fromStreamName, | ||
| 'reason' => $reasons | ||
| 'reason !=' => $leftReason, | ||
| 'insertedTime >' => $latestLeftPaidStream |
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'd recommend using >= not > just in case
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.
All requested changes done.
…ymentsInfo which return all info about payments for paid stream. (cherry picked from commit 323af27dedac8e60fb343c1080b485b9003bc0b4)
No description provided.