Skip to content

Commit

Permalink
Show Bullet, Rack Profiler and WAI WCAG tools in the footer (#12629)
Browse files Browse the repository at this point in the history
* Bump bullet from 7.0.7 to 7.1.6

* Show Bullet, Rack Profiler and WAI WCAG tools in the footer

* Skip user in notifications for Bullet

* Fix stylelint offense

* Add counter cache for forms' AnswerOptions

* Add counter cache for forms' MatrixRow

* Add counter cache for forms' DisplayConditions

* Add counter cache for forms' DisplayConditionsForOtherQuestions

* Fix specs

* Fix rubocop offenses

* Add counter cache for Attachments

As this is a polymorphic relation, we need to add it in the different
ActiveRecord models that use it.

* Roll our own counter_cache for attachments_count

The problem is basically that in the case of the DummyResource class, we
don't have a way of disabling this attribute, as it isn't a valid
attribute in the table and that's what expects counter_cache.

The solution is to have a more granular control of how the counter_cache
is updated by manually handling the increment/decrement of the fields.

* Add missing binstub for rails in decidim-forms

* Make consistent the formatting of the belongs_to in DisplayCondition model

* Revert "Add counter cache for Attachments"

This reverts commit e99d03c.

* Revert "Roll our own counter_cache for attachments_count"

This reverts commit d8ea6ea.

* Remove counter cache from attached_to relation

* Fix bad merge

* Ignore counter_cache in Bullet for proposals' attachments
  • Loading branch information
andreslucena committed May 10, 2024
1 parent 39c4749 commit 03d539e
Show file tree
Hide file tree
Showing 22 changed files with 126 additions and 15 deletions.
4 changes: 2 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ PATH
decidim-design (0.29.0.dev)
decidim-core (= 0.29.0.dev)
decidim-dev (0.29.0.dev)
bullet (~> 7.0, < 7.1.0)
bullet (~> 7.1.6)
byebug (~> 11.0)
capybara (~> 3.39)
decidim (= 0.29.0.dev)
Expand Down Expand Up @@ -282,7 +282,7 @@ GEM
brakeman (6.1.0)
browser (2.7.1)
builder (3.2.4)
bullet (7.0.7)
bullet (7.1.6)
activesupport (>= 3.0.0)
uniform_notifier (~> 1.11)
byebug (11.1.3)
Expand Down
2 changes: 2 additions & 0 deletions decidim-dev/app/packs/stylesheets/decidim/dev.scss
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
@import "stylesheets/decidim/dev/accessibility";
@import "stylesheets/decidim/dev/bullet";
@import "stylesheets/decidim/dev/rack_profiler";
@import "stylesheets/decidim/dev/map";
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
position: fixed;
display: flex;
z-index: 9999;
top: 60px;
bottom: 0;
left: 0;
background: #fff;
border: 1px solid #888;
Expand Down
4 changes: 4 additions & 0 deletions decidim-dev/app/packs/stylesheets/decidim/dev/_bullet.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
details#bullet-footer {
z-index: 77777 !important;
bottom: 48px !important;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.profiler-results.profiler-bottom {
bottom: 26px !important;
z-index: 88888 !important;
}
2 changes: 1 addition & 1 deletion decidim-dev/decidim-dev.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Gem::Specification.new do |s|
s.add_dependency "factory_bot_rails", "~> 6.2"
s.add_dependency "faker", "~> 3.2"

s.add_dependency "bullet", "~> 7.0", "< 7.1.0"
s.add_dependency "bullet", "~> 7.1.6"
s.add_dependency "byebug", "~> 11.0"
s.add_dependency "erb_lint", "~> 0.4.0"
s.add_dependency "i18n-tasks", "~> 1.0"
Expand Down
4 changes: 4 additions & 0 deletions decidim-dev/lib/decidim/dev/test/rspec_support/bullet.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
Bullet.unused_eager_loading_enable = Decidim::Env.new("DECIDIM_BULLET_UNUSED_EAGER", "false").present?
# Detect unnecessary COUNT queries which could be avoided with a counter_cache
Bullet.counter_cache_enable = Decidim::Env.new("DECIDIM_BULLET_COUNTER_CACHE", "true").present?

# Having a counter cache for this column is too difficult, as this should also work for the Decidim::DummyResource
# model, and counter_cache needs a real table in the database.
Bullet.add_safelist type: :counter_cache, class_name: "Decidim::Proposals::Proposal", association: :attachments
end

if Bullet.enable?
Expand Down
2 changes: 1 addition & 1 deletion decidim-forms/app/models/decidim/forms/answer_option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class AnswerOption < Forms::ApplicationRecord

translatable_fields :body

belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id"
belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id", counter_cache: true

has_many :display_conditions,
class_name: "DisplayCondition",
Expand Down
12 changes: 10 additions & 2 deletions decidim-forms/app/models/decidim/forms/display_condition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,18 @@ class DisplayCondition < Forms::ApplicationRecord
enum condition_type: [:answered, :not_answered, :equal, :not_equal, :match], _prefix: true

# Question which will be displayed or hidden
belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id", inverse_of: :display_conditions
belongs_to :question,
class_name: "Question",
foreign_key: "decidim_question_id",
inverse_of: :display_conditions,
counter_cache: :display_conditions_count

# Question the answers of which are checked against conditions
belongs_to :condition_question, class_name: "Question", foreign_key: "decidim_condition_question_id", inverse_of: :display_conditions_for_other_questions
belongs_to :condition_question,
class_name: "Question",
foreign_key: "decidim_condition_question_id",
inverse_of: :display_conditions_for_other_questions,
counter_cache: :display_conditions_for_other_questions_count

# Answer option provided to check for "equal" or "not_equal" (optional)
belongs_to :answer_option, class_name: "AnswerOption", foreign_key: "decidim_answer_option_id", optional: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class QuestionMatrixRow < Forms::ApplicationRecord
include Decidim::TranslatableResource

translatable_fields :body
belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id"
belongs_to :question, class_name: "Question", foreign_key: "decidim_question_id", counter_cache: :matrix_rows_count

delegate :answer_options, :mandatory, :max_choices, to: :question

Expand Down
15 changes: 15 additions & 0 deletions decidim-forms/bin/rails
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

# This command will automatically be run when you run "rails" with Rails gems
# installed from the root of your application.

ENGINE_ROOT = File.expand_path("..", __dir__)
ENGINE_PATH = File.expand_path("../lib/decidim/forms/engine", __dir__)

# Set up gems listed in the Gemfile.
ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", __dir__)
require "bundler/setup"

require "rails/all"
require "rails/engine/commands"
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class AddAnswerOptionsCounterCacheToQuestions < ActiveRecord::Migration[6.1]
def change
add_column :decidim_forms_questions, :answer_options_count, :integer, null: false, default: 0

reversible do |dir|
dir.up do
Decidim::Forms::Question.reset_column_information
Decidim::Forms::Question.find_each do |record|
record.class.reset_counters(record.id, :answer_options)
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class AddMatrixRowCounterCacheToQuestions < ActiveRecord::Migration[6.1]
def change
add_column :decidim_forms_questions, :matrix_rows_count, :integer, null: false, default: 0

reversible do |dir|
dir.up do
Decidim::Forms::Question.reset_column_information
Decidim::Forms::Question.find_each do |record|
record.class.reset_counters(record.id, :matrix_rows)
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class AddDisplayConditionsCounterCacheToQuestions < ActiveRecord::Migration[6.1]
def change
add_column :decidim_forms_questions, :display_conditions_count, :integer, null: false, default: 0

reversible do |dir|
dir.up do
Decidim::Forms::Question.reset_column_information
Decidim::Forms::Question.find_each do |record|
record.class.reset_counters(record.id, :display_conditions)
end
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class AddDisplayConditionsForOtherQuestionsCounterCacheToQuestions < ActiveRecord::Migration[6.1]
def change
add_column :decidim_forms_questions, :display_conditions_for_other_questions_count, :integer, null: false, default: 0

reversible do |dir|
dir.up do
Decidim::Forms::Question.reset_column_information
Decidim::Forms::Question.find_each do |record|
record.class.reset_counters(record.id, :display_conditions_for_other_questions)
end
end
end
end
end
4 changes: 2 additions & 2 deletions decidim-generators/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ PATH
decidim-design (0.29.0.dev)
decidim-core (= 0.29.0.dev)
decidim-dev (0.29.0.dev)
bullet (~> 7.0, < 7.1.0)
bullet (~> 7.1.6)
byebug (~> 11.0)
capybara (~> 3.39)
decidim (= 0.29.0.dev)
Expand Down Expand Up @@ -282,7 +282,7 @@ GEM
brakeman (6.1.0)
browser (2.7.1)
builder (3.2.4)
bullet (7.0.7)
bullet (7.1.6)
activesupport (>= 3.0.0)
uniform_notifier (~> 1.11)
byebug (11.1.3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
Bullet.enable = true
Bullet.bullet_logger = true
Bullet.rails_logger = true
Bullet.add_footer = true
Bullet.skip_user_in_notification = true
Bullet.stacktrace_includes = %w(decidim-)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
# initialization is skipped so trigger it
Rack::MiniProfilerRails.initialize!(Rails.application)
Rack::MiniProfiler.config.skip_paths << "/favicon.ico"
Rack::MiniProfiler.config.position = "bottom-left"
end
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def serialize
user_endorsements:
},
comments: proposal.comments_count,
attachments: proposal.attachments.count,
attachments: proposal.attachments.size,
followers: proposal.follows.size,
published_at: proposal.published_at,
url:,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module Decidim::Surveys
answer_options << answer_option.attributes
end
question_attrs[:answer_options] = answer_options
question_attrs = question_attrs.reject { |key, _value| key.ends_with?("_count") }
questions << question_attrs
end
questionnaire_attrs[:questions] = questions
Expand Down Expand Up @@ -72,7 +73,7 @@ def imported_questions_should_eq_serialized(imported_questions)
end

def imported_question_options_should_eq_serialized(imported_answer_options, original_answer_options)
expect(imported_answer_options.count).to eq(original_answer_options.count)
expect(imported_answer_options.size).to eq(original_answer_options.size)
imported_answer_options.zip(original_answer_options).each do |imported, original|
expect(imported.body).to eq(original.body)
expect(imported.free_text).to eq(original.free_text)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ def copy_questionnaire_questions(original_questionnaire, new_questionnaire)
original_questionnaire.questions.each do |original_question|
new_question = original_question.dup
new_question.questionnaire = new_questionnaire
new_question.assign_attributes(
answer_options_count: 0,
matrix_rows_count: 0,
display_conditions_count: 0,
display_conditions_for_other_questions_count: 0
)
new_question.save!
copy_questionnaire_answer_options(original_question, new_question)
copy_questionnaire_matrix_rows(original_question, new_question)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def check_copy_questionnaire_questions(source_questionnaire, new_questionnaire)
end

def check_answer_options(source_question, new_question)
expect(source_question.answer_options.count).to eq(new_question.answer_options.count)
expect(source_question.answer_options.size).to eq(new_question.answer_options.size)

source_question.answer_options.each_with_index do |source_answer_option, idx|
new_answer_option = new_question.answer_options[idx]
Expand All @@ -41,7 +41,7 @@ def check_answer_options(source_question, new_question)
end

def check_matrix_rows(source_question, new_question)
expect(source_question.matrix_rows.count).to eq(new_question.matrix_rows.count)
expect(source_question.matrix_rows.size).to eq(new_question.matrix_rows.size)

source_question.matrix_rows.each_with_index do |source_matrix_row, idx|
new_matrix_row = new_question.matrix_rows[idx]
Expand All @@ -52,7 +52,7 @@ def check_matrix_rows(source_question, new_question)
end

def check_display_conditions(source_display_conditions, new_display_conditions)
expect(source_display_conditions.count).to eq(new_display_conditions.count)
expect(source_display_conditions.size).to eq(new_display_conditions.size)

source_display_conditions.each_with_index do |source_matrix_row, idx|
new_matrix_row = new_display_conditions[idx]
Expand Down

0 comments on commit 03d539e

Please sign in to comment.