fix: "Barcode limit: 13 digits or more?" by aadarsh-ram · Pull Request #86 · openfoodfacts/folksonomy_api

aadarsh-ram · 2022-04-04T16:36:00Z

What

Folksonomy Engine recognizes products with 1-13 digit barcodes (regexp: [0-9]{1,13}) only.
As Folksonomy Engine can be used for other types of products, it must recognize barcodes with more than 13 digits.
This pull request aims to fix the above issue, as well as some other extra bugs found during testing.

Related issue(s)

Fixes barcode limit: 13 digits or more? #49
No checks done for empty product barcode.

File additions/deletions

Added a folder for database migration scripts. This folder can be used for future migration scripts as well.
Added a main migration script and a migration step file. Data migration is done using yoyo-migrations which has been added to the dependencies of Folksonomy Engine.
Added provisions for mentioning PostgresSQL user and host in local_settings_example.py
Changed regexp to check for barcodes with greater than 13 digits.
Changed table definitions of folksonomy and folksonomy_versions to include product barcodes of greater than 13 digits.
A barcode limit of 24 digits has been established after discussion.
Added a test with a barcode of 25 digits.
Removed the test which checks for greater than 14 digits barcodes.
Updated documentation

Part of

🎯 What can I work on ? (Folksonomy Engine) #70

All unit tests which are part of test_main.py have passed.
Please do provide feedback after reviewing this PR. I hope that I will contribute more to OpenFoodFacts!

…ull check

alexgarel

LGTM, but I would like @CharlesNepote or @cquest to give their opinion.

db/migrations/change-barcodedef.py

db/db-migration.py

folksonomy/api.py

alexgarel

I added some remarks.

See in particular my comment for local_settings.py if it's not clear I may explain better.

db/db-migration.py

db/db_settings.py

folksonomy/models.py

local_settings_example.py

…onomy_api into change-barcode

db/db_setup.sql

folksonomy/models.py

folksonomy/api.py

tests/test_main.py

alexgarel · 2022-04-08T13:17:22Z

@aadarsh-ram : to finish this request, I would say: just add a limit to barcode (32 is good I think).

folksonomy/models.py

db-migration.py

db/db_setup.sql

local_settings_example.py

INSTALL.md

db-migration.py

local_settings_example.py

alexgarel

Sorry @aadarsh-ram still small comments, but we are very near the end !

alexgarel

Kudos @aadarsh-ram, good work.

CharlesNepote · 2023-04-11T22:40:44Z

@aadarsh-ram @alexgarel : what should be the upgrade process then?
I started to document it here: https://github.com/openfoodfacts/openfoodfacts-infrastructure/blob/develop/docs/folksonomy.md#upgrade

But I guess I need to run a script for the DB migration?

CharlesNepote · 2023-04-12T14:03:35Z

@aadarsh-ram I added the https://github.com/openfoodfacts/folksonomy_api/blob/main/yoyo.ini file and wrote the upgrade documentation for our infrastructure: https://github.com/openfoodfacts/openfoodfacts-infrastructure/blob/develop/docs/folksonomy.md#upgrade

aadarsh-ram · 2023-04-12T14:39:22Z

@aadarsh-ram I added the https://github.com/openfoodfacts/folksonomy_api/blob/main/yoyo.ini file and wrote the upgrade documentation for our infrastructure: https://github.com/openfoodfacts/openfoodfacts-infrastructure/blob/develop/docs/folksonomy.md#upgrade

LGTM. FYI, you can use the command line interface or use the db_migrations.py script after setting the local_settings.py file.

aadarsh-ram added 3 commits April 2, 2022 15:45

Changed regex and table definitions. Added tests and db-migration script

c101c6b

Added version checks for put and post methods. Added product code = n…

22f5418

…ull check

Merge branch 'openfoodfacts:main' into change-barcode

f7f82c9

aadarsh-ram requested a review from a team as a code owner April 4, 2022 16:36

teolemon requested review from CharlesNepote and cquest April 4, 2022 16:50

teolemon assigned aadarsh-ram Apr 4, 2022

teolemon changed the title ~~Fix for "Barcode limit: 13 digits or more?"~~ Fix: "Barcode limit: 13 digits or more?" Apr 4, 2022

teolemon changed the title ~~Fix: "Barcode limit: 13 digits or more?"~~ fix: "Barcode limit: 13 digits or more?" Apr 4, 2022

teolemon and others added 2 commits April 4, 2022 18:54

Merge branch 'main' into change-barcode

537ab13

Minor unit test fix

9fc91c6

alexgarel reviewed Apr 4, 2022

View reviewed changes

db/migrations/change-barcodedef.py Show resolved Hide resolved

db/db-migration.py Outdated Show resolved Hide resolved

aadarsh-ram mentioned this pull request Apr 4, 2022

barcode limit: 13 digits or more? #49

Closed

alexgarel reviewed Apr 5, 2022

View reviewed changes

folksonomy/api.py Outdated Show resolved Hide resolved

Removed version checks in API code. Added db-settings file

872715d

alexgarel requested changes Apr 6, 2022

View reviewed changes

aadarsh-ram added 2 commits April 6, 2022 17:11

Integrated db-settings with local-settings. Pythonic null checks done

d627d3c

Merge branch 'openfoodfacts:main' into change-barcode

06692a1

aadarsh-ram requested a review from alexgarel April 6, 2022 11:48

aadarsh-ram added 3 commits April 6, 2022 20:50

Update docs and regex

4266065

Merge branch 'change-barcode' of https://github.com/aadarsh-ram/folks…

218e546

…onomy_api into change-barcode

Minor addition in models.py

ed6d964

cquest requested changes Apr 6, 2022

View reviewed changes

db/db_setup.sql Outdated Show resolved Hide resolved

folksonomy/models.py Outdated Show resolved Hide resolved

cquest requested changes Apr 6, 2022

View reviewed changes

folksonomy/api.py Outdated Show resolved Hide resolved

tests/test_main.py Show resolved Hide resolved

CharlesNepote reviewed Apr 8, 2022

View reviewed changes

folksonomy/models.py Outdated Show resolved Hide resolved

aadarsh-ram added 2 commits April 8, 2022 20:53

Merge branch 'main' into change-barcode

d43667c

Docs/Error language fix

745f783

alexgarel reviewed Apr 13, 2022

View reviewed changes

db-migration.py Show resolved Hide resolved

db/db_setup.sql Outdated Show resolved Hide resolved

local_settings_example.py Outdated Show resolved Hide resolved

Added limit of 24. Changed tests and variables.

0876ca0

aadarsh-ram requested review from CharlesNepote, alexgarel and cquest April 13, 2022 09:46

alexgarel reviewed Apr 13, 2022

View reviewed changes

INSTALL.md Outdated Show resolved Hide resolved

alexgarel reviewed Apr 13, 2022

View reviewed changes

db-migration.py Outdated Show resolved Hide resolved

alexgarel reviewed Apr 13, 2022

View reviewed changes

local_settings_example.py Outdated Show resolved Hide resolved

alexgarel requested changes Apr 13, 2022

View reviewed changes

aadarsh-ram added 2 commits April 13, 2022 21:29

Minor naming changes

c382d3b

Bring env var to same line

d26974d

aadarsh-ram requested a review from alexgarel April 13, 2022 16:09

Changed regexp for 24 digits

ac1567a

alexgarel approved these changes Apr 14, 2022

View reviewed changes

alexgarel merged commit 2f33a80 into openfoodfacts:main Apr 15, 2022

github-actions bot mentioned this pull request Apr 15, 2022

chore(main): release 1.0.0 #88

Merged

Uh oh!

Conversation

aadarsh-ram commented Apr 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Related issue(s)

File additions/deletions

Part of

Uh oh!

alexgarel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexgarel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexgarel commented Apr 8, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexgarel left a comment

Choose a reason for hiding this comment

Uh oh!

alexgarel left a comment

Choose a reason for hiding this comment

Uh oh!

CharlesNepote commented Apr 11, 2023

Uh oh!

CharlesNepote commented Apr 12, 2023

Uh oh!

aadarsh-ram commented Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aadarsh-ram commented Apr 4, 2022 •

edited

Loading

aadarsh-ram commented Apr 12, 2023 •

edited

Loading