Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix for issue 52 #56

Closed
wants to merge 2 commits into from
Closed

[WIP] Fix for issue 52 #56

wants to merge 2 commits into from

Conversation

nomaster
Copy link
Member

@nomaster nomaster commented Nov 4, 2017

The current use of the 'id' column to store barcode values is exotic at best and leads to several issues. I found no other solution than to re-create the table.

Also, I implemented the one-to-one relation of drink and barcode properly.

Barcode data will be lost upon migration, but I guess we can live with that.

Fixes #52

The property `id` is not generated by the client upon object creation.
Thus, it cannot be a requirement.
@nomaster nomaster changed the title Fix for issue 52 [WIP] Fix for issue 52 Nov 4, 2017
@msteinhoff
Copy link
Contributor

The current use of the 'id' column to store barcode values is exotic at best

Could you please elaborate this? Why do you think this is exotic?

and leads to several issues. I found no other solution than to re-create the table.

Would you describe the issues you faced?

Also, I implemented the one-to-one relation of drink and barcode properly.

What problem are you trying to solve with this change?

Barcode data will be lost upon migration, but I guess we can live with that.

Who is going to re-scan the barcodes? Why not first create the new table, copying data using INSERT INTO ... SELECT FROM ... and then dropping the old table?

@msteinhoff
Copy link
Contributor

msteinhoff commented Nov 10, 2017

Could you please elaborate this? Why do you think this is exotic?

For the record: By convention, RoR ActiveRecord tables always have a integer primary key id column. Re-using this for business values will confuse other developers.

I'd also suggest to re-write the migrations so we dont have to re-import the data:

  1. create table barcodes_new
  2. copy data from barcodes using INSERT INTO ... SELECT FROM ... (or ruby ORM)
  3. drop table barcodes
  4. rename table barcodes_new to barcodes

I'd also suggest to validate the barcode value using something like this: https://github.com/hmasing/gtin

@nomaster
Copy link
Member Author

Closing due to inactivity

@nomaster nomaster closed this Oct 30, 2020
@nomaster nomaster deleted the issue-52 branch March 22, 2021 09:23
@nomaster nomaster restored the issue-52 branch March 22, 2021 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exception when creating a barcode without an id
2 participants