MF-916 - Fix Things and Channels counters #947
Conversation
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #947 +/- ##
==========================================
- Coverage 84.07% 83.63% -0.45%
==========================================
Files 75 75
Lines 5383 5248 -135
==========================================
- Hits 4526 4389 -137
- Misses 588 590 +2
Partials 269 269
Continue to review full report at Codecov.
|
things/postgres/channels.go
Outdated
| return things.ChannelsPage{}, err | ||
| switch metadata { | ||
| case nil: | ||
| cq = fmt.Sprintf("%s %s", cq, ";") |
There was a problem hiding this comment.
You can write it like this: cq = fmt.Sprintf("%s;", cq)
things/postgres/things.go
Outdated
| return things.ThingsPage{}, err | ||
| switch metadata { | ||
| case nil: | ||
| cq = fmt.Sprintf("%s %s", cq, ";") |
There was a problem hiding this comment.
Same here, you can write it as cq = fmt.Sprintf("%s;", cq)
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
things/postgres/channels.go
Outdated
| switch name { | ||
| case "": | ||
| if err := cr.db.GetContext(ctx, &total, q, owner); err != nil { | ||
| return things.ChannelsPage{}, err | ||
| switch metadata { |
There was a problem hiding this comment.
this looks like lot of cases
cannot this be done like here
https://github.com/mainflux/mainflux/blob/22f1e851cd334f30c33f314eff76f6af23088425/things/postgres/channels.go#L125 using getNameQuery and getMetadataQuery
is total different than number of rows we get from the query above?
There was a problem hiding this comment.
@mteodor The first one returns you one page after to apply filters. The second one counts the total of Things with a certain filter in all pages.
There was a problem hiding this comment.
and this would not work
cq := fmt.Sprintf(`SELECT COUNT(*) FROM things WHERE owner = $1 %s%s`, nq, mq) ?
There was a problem hiding this comment.
I don't see how here: https://godoc.org/github.com/jmoiron/sqlx#NamedQueryContext
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
things/postgres/channels.go
Outdated
| Channel string `db:"channel"` | ||
| Thing string `db:"thing"` | ||
| Owner string `db:"owner"` | ||
| func (cr channelRepository) total(ctx context.Context, query string, params map[string]interface{}) (uint64, error) { |
There was a problem hiding this comment.
Can you create single helper function for this? (maybe this method can receive db as parameter as well)
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
things/postgres/channels.go
Outdated
| q = fmt.Sprintf(`SELECT COUNT(*) FROM channels WHERE owner = $1 %s;`, cq) | ||
|
|
||
| total := uint64(0) | ||
| cq := `SELECT COUNT(*) FROM channels WHERE owner = :owner` |
There was a problem hiding this comment.
I would do it similar like above since we alread have subqueries for metadata and name
cq := fmt.Sprintf(`SELECT COUNT(*) FROM channels WHERE owner = :owner %s%s`, nq, mq )
and no need for case at all
There was a problem hiding this comment.
You're right! I would even say that we don't need to do this checks since this name and metadata queries are returned by getNameQuery and getMetadatQuery
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
things/postgres/channels.go
Outdated
| @@ -418,7 +412,7 @@ func getNameQuery(name string) (string, string) { | |||
| nq := "" | |||
| if name != "" { | |||
| name = fmt.Sprintf(`%%%s%%`, name) | |||
| nq = ` AND LOWER(name) LIKE :name` | |||
| nq = ` AND name LIKE :name` | |||
There was a problem hiding this comment.
actually we need LOWER here, sorry
Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
* MF-916 - Fix Things and Channels counters Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Use NamedQueryContext to count rows Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Create single helper total() func Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Fix reviews Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Rm useless LOWER Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com> * Revert previous commit Signed-off-by: Manuel Imperiale <manuel.imperiale@gmail.com>
Signed-off-by: Manuel Imperiale manuel.imperiale@gmail.com
Closes #916