From 43d2c876e2991a18cefecc74c4d6ae57e47ea333 Mon Sep 17 00:00:00 2001 From: "Andrew W. Lee" Date: Sat, 24 Jun 2023 16:20:24 -0700 Subject: [PATCH] Add support for table comments (#50) This PR makes the following changes: * Adds support for table comments * Adds 2 new options keys: `with_column_comments` and `with_table_comments` * Checks for `with_comment: true, with_column_comments: true` to add column comments * Checks for `with_comment: true, with_table_comments: true` to add table comments * Changes `Options#load_defaults`, it defaults `with_column_comments` and `with_table_comments` to use the value set in `with_comment` if not previously set --- dummyapp/Gemfile.lock | 2 +- .../model_annotator/annotation_builder.rb | 18 +- .../column_annotation/annotation_builder.rb | 3 +- .../model_annotator/model_wrapper.rb | 12 +- lib/annotate_rb/options.rb | 12 +- lib/annotate_rb/parser.rb | 25 +++ .../annotation_builder_spec.rb | 178 +++++++++++++++++- .../annotation_builder_spec.rb | 83 +++++++- spec/lib/annotate_rb/options_spec.rb | 45 ++++- spec/lib/annotate_rb/parser_spec.rb | 45 +++++ spec/support/annotate_test_helpers.rb | 24 +++ 11 files changed, 429 insertions(+), 18 deletions(-) diff --git a/dummyapp/Gemfile.lock b/dummyapp/Gemfile.lock index d1660ddd..30e2b3ec 100644 --- a/dummyapp/Gemfile.lock +++ b/dummyapp/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: .. specs: - annotaterb (4.3.0) + annotaterb (4.3.1) GEM remote: https://rubygems.org/ diff --git a/lib/annotate_rb/model_annotator/annotation_builder.rb b/lib/annotate_rb/model_annotator/annotation_builder.rb index f23d2494..63431939 100644 --- a/lib/annotate_rb/model_annotator/annotation_builder.rb +++ b/lib/annotate_rb/model_annotator/annotation_builder.rb @@ -70,11 +70,11 @@ def schema_header_text info << "#" if @options[:format_markdown] - info << "# Table name: `#{@model.table_name}`" + info << "# Table name: `#{table_name}`" info << "#" info << "# ### Columns" else - info << "# Table name: #{@model.table_name}" + info << "# Table name: #{table_name}" end info << "#\n" # We want the last line break @@ -94,6 +94,20 @@ def schema_footer_text info.join("\n") end + + private + + def table_name + table_name = @model.table_name + display_table_comments = @options[:with_comment] && @options[:with_table_comments] + + if display_table_comments && @model.has_table_comments? + table_comment = "(#{@model.table_comments.gsub(/\n/, "\\n")})" + table_name = "#{table_name}#{table_comment}" + end + + table_name + end end end end diff --git a/lib/annotate_rb/model_annotator/column_annotation/annotation_builder.rb b/lib/annotate_rb/model_annotator/column_annotation/annotation_builder.rb index 19d0421d..98c82314 100644 --- a/lib/annotate_rb/model_annotator/column_annotation/annotation_builder.rb +++ b/lib/annotate_rb/model_annotator/column_annotation/annotation_builder.rb @@ -26,7 +26,8 @@ def build column_attributes = AttributesBuilder.new(@column, @options, is_primary_key, column_indices, column_defaults).build formatted_column_type = TypeBuilder.new(@column, @options, column_defaults).build - col_name = if @model.with_comments? && @column.comment + display_column_comments = @options[:with_comment] && @options[:with_column_comments] + col_name = if display_column_comments && @model.with_comments? && @column.comment "#{@column.name}(#{@column.comment.gsub(/\n/, '\\n')})" else @column.name diff --git a/lib/annotate_rb/model_annotator/model_wrapper.rb b/lib/annotate_rb/model_annotator/model_wrapper.rb index bc9f863c..83698cf2 100644 --- a/lib/annotate_rb/model_annotator/model_wrapper.rb +++ b/lib/annotate_rb/model_annotator/model_wrapper.rb @@ -49,6 +49,15 @@ def table_exists? @klass.table_exists? end + def table_comments + @klass.connection.table_comment(@klass.table_name) + end + + def has_table_comments? + @klass.connection.respond_to?(:table_comment) && + @klass.connection.table_comment(@klass.table_name).present? + end + def column_defaults @klass.column_defaults end @@ -110,8 +119,7 @@ def retrieve_indexes_from_table end def with_comments? - @with_comments ||= @options[:with_comment] && - raw_columns.first.respond_to?(:comment) && + @with_comments ||= raw_columns.first.respond_to?(:comment) && raw_columns.map(&:comment).any? { |comment| !comment.nil? } end diff --git a/lib/annotate_rb/options.rb b/lib/annotate_rb/options.rb index 623517ab..4ca1764a 100644 --- a/lib/annotate_rb/options.rb +++ b/lib/annotate_rb/options.rb @@ -51,7 +51,9 @@ def from(options = {}, state = {}) sort: false, # ModelAnnotator timestamp: false, # RouteAnnotator trace: false, # ModelAnnotator, but is part of Core - with_comment: true # ModelAnnotator + with_comment: true, # ModelAnnotator + with_column_comments: nil, # ModelAnnotator + with_table_comments: nil # ModelAnnotator }.freeze OTHER_OPTIONS = { @@ -113,7 +115,9 @@ def from(options = {}, state = {}) :sort, :timestamp, :trace, - :with_comment + :with_comment, + :with_column_comments, + :with_table_comments ].freeze OTHER_OPTION_KEYS = [ @@ -187,6 +191,10 @@ def load_defaults @options[:wrapper_open] ||= @options[:wrapper] @options[:wrapper_close] ||= @options[:wrapper] + # Set column and table comments to default to :with_comment, if not set + @options[:with_column_comments] = @options[:with_comment] if @options[:with_column_comments].nil? + @options[:with_table_comments] = @options[:with_comment] if @options[:with_table_comments].nil? + self end diff --git a/lib/annotate_rb/parser.rb b/lib/annotate_rb/parser.rb index bafa8836..84c240d2 100644 --- a/lib/annotate_rb/parser.rb +++ b/lib/annotate_rb/parser.rb @@ -217,6 +217,31 @@ def add_model_options_to_parser(option_parser) "include database comments in model annotations") do @options[:with_comment] = true end + + option_parser.on("--without-comment", + "include database comments in model annotations") do + @options[:with_comment] = false + end + + option_parser.on("--with-column-comments", + "include column comments in model annotations") do + @options[:with_column_comments] = true + end + + option_parser.on("--without-column-comments", + "exclude column comments in model annotations") do + @options[:with_column_comments] = false + end + + option_parser.on("--with-table-comments", + "include table comments in model annotations") do + @options[:with_table_comments] = true + end + + option_parser.on("--without-table-comments", + "exclude table comments in model annotations") do + @options[:with_table_comments] = false + end end def add_route_options_to_parser(option_parser) diff --git a/spec/lib/annotate_rb/model_annotator/annotation_builder_spec.rb b/spec/lib/annotate_rb/model_annotator/annotation_builder_spec.rb index 227dd326..b21d72b3 100644 --- a/spec/lib/annotate_rb/model_annotator/annotation_builder_spec.rb +++ b/spec/lib/annotate_rb/model_annotator/annotation_builder_spec.rb @@ -1319,7 +1319,7 @@ end let :options do - AnnotateRb::Options.new({classified_sort: false, with_comment: true}) + AnnotateRb::Options.new({classified_sort: false, with_comment: true, with_column_comments: true}) end let :columns do @@ -1358,7 +1358,7 @@ end let :options do - AnnotateRb::Options.new({classified_sort: false, with_comment: true}) + AnnotateRb::Options.new({classified_sort: false, with_comment: true, with_column_comments: true}) end let :columns do @@ -1405,7 +1405,7 @@ end let :options do - AnnotateRb::Options.new({classified_sort: false, with_comment: true}) + AnnotateRb::Options.new({classified_sort: false, with_comment: true, with_column_comments: true}) end let :columns do @@ -1851,7 +1851,7 @@ end let :options do - {format_rdoc: true, with_comment: true} + {format_rdoc: true, with_comment: true, with_column_comments: true} end let :columns do @@ -1899,7 +1899,7 @@ end let :options do - {format_markdown: true, with_comment: true} + {format_markdown: true, with_comment: true, with_column_comments: true} end let :columns do @@ -1968,4 +1968,172 @@ end end end + + describe "#schema_header_text" do + subject do + described_class.new(klass, options).schema_header_text + end + + let(:table_exists) { true } + let(:table_comment) { "" } + + let(:connection) do + indexes = [] + foreign_keys = [] + + mock_connection_with_table_fields( + indexes, + foreign_keys, + table_exists, + table_comment + ) + end + + let :klass do + primary_key = nil + columns = [] + + mock_class_with_custom_connection( + :users, + primary_key, + columns, + connection + ) + end + + context "with no options set" do + let :options do + AnnotateRb::Options.new({}) + end + + let(:expected_header) do + <<~HEADER + # + # Table name: users + # + HEADER + end + + it "returns the schema header" do + is_expected.to eq(expected_header) + end + end + + context "with `with_comment: true`" do + context "with `with_table_comments: true` and table has comments" do + let :options do + AnnotateRb::Options.new({with_comment: true, with_table_comments: true}) + end + + let(:table_comment) { "table_comments" } + + let(:expected_header) do + <<~HEADER + # + # Table name: users(table_comments) + # + HEADER + end + + it "returns the header with the table comment" do + is_expected.to eq(expected_header) + end + end + + context "with `with_table_comments: true` and table does not have comments" do + let :options do + AnnotateRb::Options.new({with_comment: true, with_table_comments: true}) + end + + let :klass do + primary_key = nil + columns = [] + indexes = [] + foreign_keys = [] + + mock_class( + :users, + primary_key, + columns, + indexes, + foreign_keys + ) + end + + let(:expected_header) do + <<~HEADER + # + # Table name: users + # + HEADER + end + + it "returns the header without table comments" do + is_expected.to eq(expected_header) + end + end + + context "with `with_table_comments: false` and table has comments" do + let :options do + AnnotateRb::Options.new({with_comment: true, with_table_comments: false}) + end + + let(:table_comment) { "table_comments" } + + let(:expected_header) do + <<~HEADER + # + # Table name: users + # + HEADER + end + + it "returns the header without the table comment" do + is_expected.to eq(expected_header) + end + end + end + + context "with `with_comment: false`" do + context "with `with_table_comments: true` and table has comments" do + let :options do + AnnotateRb::Options.new({with_comment: false, with_table_comments: true}) + end + + let(:table_comment) { "table_comments" } + + let(:expected_header) do + <<~HEADER + # + # Table name: users + # + HEADER + end + + it "returns the header without the table comment" do + is_expected.to eq(expected_header) + end + end + + context "with `with_table_comments: false` and table has comments" do + let :options do + AnnotateRb::Options.new({with_comment: false, with_table_comments: false}) + end + + let(:table_comment) { "table_comments" } + + let(:expected_header) do + <<~HEADER + # + # Table name: users + # + HEADER + end + + it "returns the header without the table comment" do + is_expected.to eq(expected_header) + end + end + end + end end diff --git a/spec/lib/annotate_rb/model_annotator/column_annotation/annotation_builder_spec.rb b/spec/lib/annotate_rb/model_annotator/column_annotation/annotation_builder_spec.rb index 88af00ae..2401e238 100644 --- a/spec/lib/annotate_rb/model_annotator/column_annotation/annotation_builder_spec.rb +++ b/spec/lib/annotate_rb/model_annotator/column_annotation/annotation_builder_spec.rb @@ -8,7 +8,7 @@ let(:max_size) { 16 } describe "bare format" do - let(:options) { AnnotateRb::Options.new({}) } + let(:options) { AnnotateRb::Options.new({with_comment: true, with_column_comments: true}) } context "when the column is the primary key" do let(:column) { mock_column("id", :integer) } @@ -77,10 +77,85 @@ is_expected.to eq(expected_result) end end + + context "when the column has a comment and without comment options" do + let(:options) { AnnotateRb::Options.new({with_comment: false, with_column_comments: false}) } + let(:max_size) { 20 } + + let(:column) { mock_column("id", :integer, comment: "[is commented]") } + let(:model) do + instance_double( + AnnotateRb::ModelAnnotator::ModelWrapper, + primary_key: "something_else", + retrieve_indexes_from_table: [], + with_comments?: true, + column_defaults: {} + ) + end + let(:expected_result) do + <<~COLUMN + # id :integer not null + COLUMN + end + + it "returns the column annotation without the comment" do + is_expected.to eq(expected_result) + end + end + + context "when the column has a comment and with `with_comment: true`" do + let(:options) { AnnotateRb::Options.new({with_comment: true, with_column_comments: false}) } + let(:max_size) { 20 } + + let(:column) { mock_column("id", :integer, comment: "[is commented]") } + let(:model) do + instance_double( + AnnotateRb::ModelAnnotator::ModelWrapper, + primary_key: "something_else", + retrieve_indexes_from_table: [], + with_comments?: true, + column_defaults: {} + ) + end + let(:expected_result) do + <<~COLUMN + # id :integer not null + COLUMN + end + + it "returns the column annotation without the comment" do + is_expected.to eq(expected_result) + end + end + + context "when the column has a comment and with `with_column_comments: true`" do + let(:options) { AnnotateRb::Options.new({with_comment: false, with_column_comments: true}) } + let(:max_size) { 20 } + + let(:column) { mock_column("id", :integer, comment: "[is commented]") } + let(:model) do + instance_double( + AnnotateRb::ModelAnnotator::ModelWrapper, + primary_key: "something_else", + retrieve_indexes_from_table: [], + with_comments?: true, + column_defaults: {} + ) + end + let(:expected_result) do + <<~COLUMN + # id :integer not null + COLUMN + end + + it "returns the column annotation without the comment" do + is_expected.to eq(expected_result) + end + end end describe "rdoc format" do - let(:options) { AnnotateRb::Options.new({format_rdoc: true}) } + let(:options) { AnnotateRb::Options.new({format_rdoc: true, with_comment: true, with_column_comments: true}) } context "when the column is the primary key" do let(:column) { mock_column("id", :integer) } @@ -162,7 +237,7 @@ end describe "yard format" do - let(:options) { AnnotateRb::Options.new({format_yard: true}) } + let(:options) { AnnotateRb::Options.new({format_yard: true, with_comment: true, with_column_comments: true}) } context "when the column is the primary key" do let(:column) { mock_column("id", :integer) } @@ -244,7 +319,7 @@ end describe "markdown format" do - let(:options) { AnnotateRb::Options.new({format_markdown: true}) } + let(:options) { AnnotateRb::Options.new({format_markdown: true, with_comment: true, with_column_comments: true}) } context "when the column is the primary key" do let(:column) { mock_column("id", :integer) } diff --git a/spec/lib/annotate_rb/options_spec.rb b/spec/lib/annotate_rb/options_spec.rb index f49b1923..806c1b55 100644 --- a/spec/lib/annotate_rb/options_spec.rb +++ b/spec/lib/annotate_rb/options_spec.rb @@ -13,10 +13,10 @@ describe ".load_defaults" do subject { described_class.new(options, state).load_defaults } - let(:key) { :show_complete_foreign_keys } let(:state) { {} } context 'when default value of "show_complete_foreign_keys" is not set' do + let(:key) { :show_complete_foreign_keys } let(:options) { {} } it "returns false" do @@ -25,6 +25,7 @@ end context 'when default value of "show_complete_foreign_keys" is set' do + let(:key) { :show_complete_foreign_keys } let(:options) { {key => true} } it "returns true" do @@ -32,6 +33,48 @@ end end + describe "comment options" do + context "when using defaults" do + let(:options) { {} } + + it "uses the defaults" do + expect(subject[:with_comment]).to eq(true) + expect(subject[:with_column_comments]).to eq(true) + expect(subject[:with_table_comments]).to eq(true) + end + end + + context 'when "with_comment" is false' do + let(:options) { {with_comment: false, with_column_comments: nil, with_table_comments: nil} } + + it 'sets "with_column_comments" and "with_table_comments"' do + expect(subject[:with_comment]).to eq(false) + expect(subject[:with_column_comments]).to eq(false) + expect(subject[:with_table_comments]).to eq(false) + end + end + + context 'when "with_column_comments" and "with_comment" set to true' do + let(:options) { {with_comment: true, with_column_comments: nil, with_table_comments: false} } + + it 'does not set "with_column_comments" to match "with_comment"' do + expect(subject[:with_comment]).to eq(true) + expect(subject[:with_column_comments]).to eq(true) + expect(subject[:with_table_comments]).to eq(false) + end + end + + context 'when "with_table_comments" and "with_comment" set to true' do + let(:options) { {with_comment: true, with_column_comments: false, with_table_comments: nil} } + + it 'does not set "with_table_comments" to match "with_comment"' do + expect(subject[:with_comment]).to eq(true) + expect(subject[:with_column_comments]).to eq(false) + expect(subject[:with_table_comments]).to eq(true) + end + end + end + describe "path options" do context "when model_dir is a string with multiple paths" do let(:options) do diff --git a/spec/lib/annotate_rb/parser_spec.rb b/spec/lib/annotate_rb/parser_spec.rb index b7ce5558..a774c9bc 100644 --- a/spec/lib/annotate_rb/parser_spec.rb +++ b/spec/lib/annotate_rb/parser_spec.rb @@ -525,5 +525,50 @@ module AnnotateRb # rubocop:disable Metrics/ModuleLength expect(result).to include(with_comment: true) end end + + describe "--without-comment" do + let(:option) { "--without-comment" } + let(:args) { [option] } + + it "sets with_comment to false" do + expect(result).to include(with_comment: false) + end + end + + describe "--with-table-comments" do + let(:option) { "--with-table-comments" } + let(:args) { [option] } + + it "sets with_table_comments to true" do + expect(result).to include(with_table_comments: true) + end + end + + describe "--without-table-comments" do + let(:option) { "--without-table-comments" } + let(:args) { [option] } + + it "sets with_table_comments to false" do + expect(result).to include(with_table_comments: false) + end + end + + describe "--with-column-comments" do + let(:option) { "--with-column-comments" } + let(:args) { [option] } + + it "sets with_column_comments to true" do + expect(result).to include(with_column_comments: true) + end + end + + describe "--without-column-comments" do + let(:option) { "--without-column-comments" } + let(:args) { [option] } + + it "sets with_column_comments to false" do + expect(result).to include(with_column_comments: false) + end + end end end diff --git a/spec/support/annotate_test_helpers.rb b/spec/support/annotate_test_helpers.rb index ace51280..3e8535ea 100644 --- a/spec/support/annotate_test_helpers.rb +++ b/spec/support/annotate_test_helpers.rb @@ -43,6 +43,15 @@ def mock_connection(indexes = [], foreign_keys = []) supports_foreign_keys?: true) end + def mock_connection_with_table_fields(indexes, foreign_keys, table_exists, table_comment) + double("Conn with table fields", + indexes: indexes, + foreign_keys: foreign_keys, + supports_foreign_keys?: true, + table_exists?: table_exists, + table_comment: table_comment) + end + def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = []) options = { connection: mock_connection(indexes, foreign_keys), @@ -58,6 +67,21 @@ def mock_class(table_name, primary_key, columns, indexes = [], foreign_keys = [] double("An ActiveRecord class", options) end + def mock_class_with_custom_connection(table_name, primary_key, columns, connection) + options = { + connection: connection, + table_exists?: true, + table_name: table_name, + primary_key: primary_key, + column_names: columns.map { |col| col.name.to_s }, + columns: columns, + column_defaults: columns.map { |col| [col.name, col.default] }.to_h, + table_name_prefix: "" + } + + double("An ActiveRecord class", options) + end + def mock_column(name, type, options = {}) default_options = { limit: nil,