Fixes panel query for the publisher #1642
Conversation
NejcZdovc
force-pushed
the
media-publisher-db-60
branch
from
964c565 to
2b50d59
Compare
| "WHERE pi.publisher_id=? AND ((ai.month = ? " | ||
| "AND ai.year = ? AND ai.reconcile_stamp = ?) OR " | ||
| "ai.percent IS NULL) LIMIT 1")); | ||
| "SELECT pi.publisher_id, pi.name, pi.url, pi.favIcon, " |
There was a problem hiding this comment.
I know this isn't the original PR; but I would recommend looking at situations where using a view makes sense (instead putting the select statement here). With a view, you can select from it like a table and there's a possibility you can re-use the view in other methods. Kind of nice because you abstract away the joins and basically just have the business logic in here
| "SELECT IFNULL(percent, 0) FROM activity_info WHERE " | ||
| "publisher_id = ? AND reconcile_stamp = ? " | ||
| ") as percent " | ||
| "FROM publisher_info AS pi WHERE pi.publisher_id = ? LIMIT 1")); | ||
|
|
||
| info_sql.BindString(0, filter.id); |
There was a problem hiding this comment.
instead of hard-coding the number, you might initialize a variable (unsigned char, unsigned short, etc) to 0 and just ++ it on each field
unsigned short field_index = 0;
info_sql.BindString(field_index++, filter.id);
info_sql.BindInt64(field_index++, filter.reconcile_stamp);
| info_sql.BindInt(2, filter.year); | ||
| info_sql.BindInt64(3, filter.reconcile_stamp); | ||
| info_sql.BindInt64(1, filter.reconcile_stamp); | ||
| info_sql.BindString(2, filter.id); |
There was a problem hiding this comment.
ID is being returned twice here; field 0 and 2. Is that correct?
There was a problem hiding this comment.
yes, we bind id in two places
There was a problem hiding this comment.
Uplift to 0.60.x approved after deliberating with @bsclifton @srirambv & @rebron 👍 Please make sure that all associated issue(s) are moved into the correct milestone.
Related PR #1589
Resolves brave/brave-browser#3329
Related issues: brave/brave-browser#3244
Related issues: brave/brave-browser#3135
Related issues: brave/brave-browser#3043
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: