feat: Sort collection API #888

Jagrutiti · 2022-09-03T08:22:03Z

What

The instruction of the PR is here:

by default, always sort on freshest item (higher id ?) first

Part of

Display insight / predictions etc for a product or a particular filter #618

codecov · 2022-09-03T08:29:41Z

Codecov Report

Merging #888 (069dabd) into master (1e63ca2) will increase coverage by 7.56%.
The diff coverage is 73.92%.

@@            Coverage Diff             @@
##           master     #888      +/-   ##
==========================================
+ Coverage   44.73%   52.29%   +7.56%     
==========================================
  Files          96       92       -4     
  Lines        6981     6958      -23     
==========================================
+ Hits         3123     3639     +516     
+ Misses       3858     3319     -539

Impacted Files	Coverage Δ
robotoff/cli/batch.py	`0.00% <ø> (ø)`
robotoff/cli/insights.py	`0.00% <0.00%> (ø)`
robotoff/insights/dataclass.py	`100.00% <ø> (ø)`
robotoff/prediction/ocr/brand.py	`68.62% <0.00%> (ø)`
robotoff/prediction/ocr/expiration_date.py	`25.71% <ø> (ø)`
robotoff/prediction/ocr/image_flag.py	`41.02% <ø> (ø)`
robotoff/prediction/ocr/label.py	`72.30% <0.00%> (ø)`
robotoff/prediction/ocr/packager_code.py	`73.07% <ø> (ø)`
robotoff/prediction/ocr/product_weight.py	`49.10% <0.00%> (+1.28%)`	⬆️
robotoff/products.py	`42.75% <ø> (+2.26%)`	⬆️
... and 61 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

alexgarel · 2022-09-05T15:47:43Z

robotoff/app/core.py

+    else:
+        query = query.order_by(ProductInsight.id.desc())


As said in meeting: better not introduce a default sorting in a widely used function, it might have unexpected side effects.

We should instead use order_by="most_recent" in Collection view and implement it here.

Also you should use timestamp here and not id, because id is special on ProductInsight (it's not numerical, it's a uuid).

alexgarel · 2022-09-05T15:49:27Z

robotoff/app/core.py

@@ -363,6 +369,8 @@ def get_logo_annotation(
    if offset is not None:
        query = query.offset(offset)

+    # query = query.order_by(LogoAnnotation.image_prediction.timestamp.desc())


here you can use LogoAnnotation.id for order_by. id exists because it's automatically added by peewee.

See https://docs.peewee-orm.com/en/latest/peewee/models.html#fields

In the above example, because none of the fields are initialized with primary_key=True, an auto-incrementing primary key will automatically be created and named “id”. Peewee uses AutoField to signify an auto-incrementing integer primary key, which implies primary_key=True.