Over promotions #6403
Over promotions #6403
Conversation
NejcZdovc
requested review from
emerick and
zenparsing
and removed request for
a team
NejcZdovc
force-pushed
the
over-promotions
branch
2 times, most recently
from
d71dce3 to
5be81bf
Compare
| } | ||
|
|
||
| found = false; | ||
| for (const auto& item : list) { |
There was a problem hiding this comment.
Another option would be something like:
bool found = std::any_of(list.begin(), list.end(), [&promotion](auto& item) {
item->id == promotion.second->id;
});But haven't we already erased all entries in promotions that have a matching id (on line 255)? Is this search required?
There was a problem hiding this comment.
yes it's needed. We only erased promotions that we received from api and we have it in db. What we are doing here is going through active promotions that we have in db and if we don't find them in api we mark them as over
There was a problem hiding this comment.
Thinking this through...
The loop starting on line 21 goes through all entries in list and removes the corresponding entry in promotions, if there is one. So after the loop completes, all entries in list have been removed from promotions, and promotions only contains entries that do not have a matching entry in list. Am I missing something?
Another way to simplify things (while avoiding a nested loop) might be to not erase from promotions, and instead populate an std::set with the ids from list, and then check that set in this loop over promotions.
There was a problem hiding this comment.
that is correct. Use case is like this. Server serves promotion and we put it in db. Promotion has status active. User doesn't claim it. Then this promotion is removed from the server. We need to remove all promotions from db that has status active, but don't have corresponding promotion from server response
vendor/bat-native-ledger/src/bat/ledger/internal/promotion/promotion.cc
Outdated
Show resolved
Hide resolved
NejcZdovc
force-pushed
the
over-promotions
branch
from
5be81bf to
eb3a5ba
Compare
Resolves brave/brave-browser#11170
Submitter Checklist:
npm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist:
After-merge Checklist:
changes has landed on.