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

Version table will use uuid as primary key type if uuid flag is set #1414

Merged
merged 2 commits into from
Aug 7, 2023

Conversation

Nuzair46
Copy link
Contributor

@Nuzair46 Nuzair46 commented Jan 10, 2023

Version table will use uuid as primary key type if uuid flag is specified on migration generation

Fix for #1402

followup to #1374

Thank you for your contribution!

Check the following boxes:

  • Wrote good commit messages.
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new
    code introduces user-observable changes.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.

@Nuzair46 Nuzair46 marked this pull request as draft January 10, 2023 13:57
@Nuzair46 Nuzair46 marked this pull request as ready for review January 10, 2023 14:12
Copy link
Member

@jaredbeck jaredbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Nuzair, looks good. Please add a changelog entry under Unreleased -> Fixed.

@Nuzair46 Nuzair46 requested a review from jaredbeck January 30, 2023 14:16
@github-actions
Copy link

github-actions bot commented May 1, 2023

This PR has been automatically marked as stale due to inactivity.
The resources of our volunteers are limited.
If this is something you are committed to continue working on, please address any concerns raised by review and/or ping us again.
Thank you for all your contributions.

@github-actions github-actions bot added the Stale label May 1, 2023
@scottengelhardt
Copy link

@jaredbeck @Nuzair46 Any updates on this? PaperTrail isn't usable without UUIDs as the item ID

@Nuzair46
Copy link
Contributor Author

Nuzair46 commented May 1, 2023

@jaredbeck Can we get this merged?
@dudescott , Papertrail will work with uuid now. This PR is just to update the version table to use uuid.
item_id will support uuid since #1374

@scottengelhardt
Copy link

Doing bundle exec rails generate paper_trail:install [--uuid] doesn't change the item_id from bigint to uuid as mentioned in the readme

@Nuzair46
Copy link
Contributor Author

Nuzair46 commented May 1, 2023

It will change to "string" type and which will allow for uuid. I don't remember why it was changed to string instead of uuid. But the change is working for me.
Im not sure if bundle exec rails generate paper_trail:install [--uuid] will update the type in already existing tables. It should be used to generate the migration during the initial setup. In this case, It should work by creating a migration to update the type of item_id to string for already existing tables.

@scottengelhardt
Copy link

Unfortunately it doesn't generate a string column and manually changing doesn't work. Regardless of the [--uuid] flag, the column stays as bigint. I'm using Version 13 or PaperTrail, 2.7.1 of Ruby and 5.2 of active record.

@scottengelhardt
Copy link

Got it. The brackets are not valid. Doing bundle exec rails generate paper_trail:install --uuid worked while bundle exec rails generate paper_trail:install [--uuid] did not.

@Nuzair46
Copy link
Contributor Author

Nuzair46 commented May 1, 2023

Yea. The documentation needs to be updated so that the commands are shown properly. It is a bit confusing right now.

@github-actions github-actions bot removed the Stale label May 2, 2023
@Nuzair46
Copy link
Contributor Author

@jaredbeck this pr has been open for months. can it be merged?

@jaredbeck jaredbeck force-pushed the version-table-with-uuid branch from 49b4430 to f71c88d Compare August 7, 2023 00:16
…fied on migration generation

removed unwanted changes

fixed ci test

reset gemspec

added changelog
@jaredbeck jaredbeck force-pushed the version-table-with-uuid branch from 95686bb to db46feb Compare August 7, 2023 01:46
@jaredbeck jaredbeck merged commit 84f81de into paper-trail-gem:master Aug 7, 2023
@jaredbeck
Copy link
Member

Doing bundle exec rails generate paper_trail:install --uuid worked while bundle exec rails generate paper_trail:install [--uuid] did not.

For future reference, the brackets indicate optionality. This is a documentation convention going back 30+ years.

@jaredbeck
Copy link
Member

Released in 15.0.0

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.

3 participants