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

Add parentheses in allowed column pattern for commented columns #1000

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

m11o
Copy link

@m11o m11o commented Dec 3, 2023

This PR adds parentheses in allowed column pattern for commented columns.

The annotation of a model with commented columns is not updated even if the column will be updated.

Steps to reproduce

  1. Create a new Rails application and use MySQL or PostgreSQL database
  2. Add annotate_models gem in Gemfile
  3. Create a model with a commented column
    • e.g.
    • create_table :users do |t|
        t.string :name, comment: 'first_name + last_name'
        t.string :email, null: false
      
        t.timestamps
      end
  4. Change the commented column from default null to non-null.
    • e.g.
    • change_column_null :users, :name, false
  5. The annotation of the model isn't updated
    • Model files unchanged.
      
    • but, schema.rb is updated.
    • -ActiveRecord::Schema[7.0].define(version: 2023_12_03_034600) do
      +ActiveRecord::Schema[7.0].define(version: 2023_12_03_040324) do
         create_table "users", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
      -    t.string "name", comment: "first_name + last_name"
      +    t.string "name", null: false, comment: "first_name + last_name"
           t.string "email", null: false
           t.datetime "created_at", null: false
           t.datetime "updated_at", null: false

the annotation of model which has commented column is not updated even if the column will be updated options of the column.
Comment on lines +3096 to +3099
change { File.read(@model_file_name) }
.from(include("name(commented column) :string\n"))
.to(include("name(commented column) :string not null\n"))
)
Copy link
Author

Choose a reason for hiding this comment

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

If parentheses aren't added, this annotation isn't updated and this test is failed with the following error.

  1) AnnotateModels annotating a file if a table has a commented column and the model already has annotation and the commented column is updated to non-null should be added non-null option in the commented column
     Failure/Error:
       expect do
         AnnotateModels.annotate_one_file(@model_file_name, schema_info, :position_in_class, @annotation_options)
       end.to change { File.read(@model_file_name) }
                .from(include("name(commented column) :string\n"))
                .to(include("name(commented column) :string           not null\n"))

       expected `File.read(@model_file_name)` to have changed from include "name(commented column) :string\n" to include "name(commented column) :string           not null\n", but did not change
     # ./spec/lib/annotate/annotate_models_spec.rb:3087:in `block (5 levels) in <top (required)>'

topalovic added a commit to zero44gmbh/annotate_models that referenced this pull request Jan 15, 2024
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.

2 participants