From 00ae27b2068a026bf56184081500202cb765352a Mon Sep 17 00:00:00 2001 From: Sebastian Noe Date: Tue, 12 May 2026 10:51:19 +0200 Subject: [PATCH] fix: resolve all lint offenses + add local CI infrastructure (#9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: resolve all Rubocop and ERBLint offenses Rubocop (16 offenses): - Style/IfUnlessModifier in account_logo_controller - Lint/RedundantSafeNavigation in templates_documents_controller - Layout/LineLength in templates_documents_controller, account_config - Rails/WhereMissing in teams_controller - Rails/WhereExists in send_submitter_reminder_email_job - Style/StringLiterals in create_teams migration - Metrics/* (disabled via inline comments for complex case statements) ERBLint (10 errors): - Void element self-closing tags (img /> → img >) - Layout/ArgumentAlignment in reminder_queue - Style/StringLiterals + Rails/LinkToBlank in navbar_buttons - Layout/BlockAlignment in custom_content mailer - Style/WordArray in role_select * feat: add local CI via Docker and pre-push lint hook - Add docker-compose.ci.yml: lint, brakeman, rspec services - Add Dockerfile.ci: test environment with Ruby, Node, Chromium - Add bin/lint: quick lint-only check - Add bin/ci: full CI suite (lint + brakeman + rspec) - Add .githooks/pre-push: auto-runs linters before push - Update docker-compose.yml: use ghcr.io image instead of local build Setup: git config core.hooksPath .githooks Usage: bin/ci or bin/lint --------- Co-authored-by: Sebastian Noe --- .githooks/pre-push | 14 +++++ Dockerfile.ci | 29 +++++++++ app/controllers/account_logo_controller.rb | 4 +- .../api/templates_documents_controller.rb | 5 +- app/controllers/teams_controller.rb | 2 +- app/controllers/templates_clone_controller.rb | 2 +- app/jobs/process_submitter_reminders_job.rb | 2 +- app/jobs/send_submitter_reminder_email_job.rb | 2 +- app/models/account_config.rb | 20 ++++-- .../_reminder_queue.html.erb | 6 +- .../_logo_form.html.erb | 2 +- app/views/shared/_navbar_buttons.html.erb | 2 +- app/views/start_form/_docuseal_logo.html.erb | 2 +- app/views/submissions/_logo.html.erb | 2 +- app/views/submit_form/_docuseal_logo.html.erb | 2 +- .../submitter_mailer/_custom_content.html.erb | 6 +- app/views/users/_role_select.html.erb | 2 +- bin/ci | 31 ++++++++++ bin/lint | 12 ++++ db/migrate/20260508100000_create_teams.rb | 2 +- docker-compose.ci.yml | 61 +++++++++++++++++++ docker-compose.yml | 4 +- lib/submitter_reminders.rb | 4 +- 23 files changed, 188 insertions(+), 30 deletions(-) create mode 100755 .githooks/pre-push create mode 100644 Dockerfile.ci create mode 100755 bin/ci create mode 100755 bin/lint create mode 100644 docker-compose.ci.yml diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 00000000..b7d49c21 --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,14 @@ +#!/bin/sh +# Pre-push hook: runs linting via Docker before pushing to GitHub. +# Ensures Rubocop, ERBLint, and ESLint pass locally. +# Skip with: git push --no-verify +# +# Enable this hook: git config core.hooksPath .githooks +set -e + +echo "🔍 Running lint checks before push..." + +docker compose -f docker-compose.ci.yml build lint --quiet 2>/dev/null +docker compose -f docker-compose.ci.yml run --rm --no-deps lint + +echo "✅ All lint checks passed." diff --git a/Dockerfile.ci b/Dockerfile.ci new file mode 100644 index 00000000..d000db10 --- /dev/null +++ b/Dockerfile.ci @@ -0,0 +1,29 @@ +FROM ruby:4.0.1-alpine + +ENV RAILS_ENV=test +ENV NODE_ENV=test + +WORKDIR /app + +RUN apk add --no-cache \ + build-base \ + git \ + libpq-dev \ + yaml-dev \ + nodejs \ + yarn \ + vips-dev \ + chromium \ + chromium-chromedriver \ + && wget -O pdfium-linux.tgz "https://github.com/docusealco/pdfium-binaries/releases/latest/download/pdfium-linux-musl-$(uname -m | sed 's/x86_64/x64/;s/aarch64/arm64/').tgz" \ + && mkdir -p /pdfium && tar -xzf pdfium-linux.tgz -C /pdfium \ + && cp /pdfium/lib/libpdfium.so /usr/lib/ \ + && rm -rf pdfium-linux.tgz /pdfium + +COPY Gemfile Gemfile.lock ./ +RUN bundle install --jobs 4 --retry 3 + +COPY package.json yarn.lock ./ +RUN yarn install --frozen-lockfile + +COPY . . diff --git a/app/controllers/account_logo_controller.rb b/app/controllers/account_logo_controller.rb index c58ad971..f3174c77 100644 --- a/app/controllers/account_logo_controller.rb +++ b/app/controllers/account_logo_controller.rb @@ -6,9 +6,7 @@ class AccountLogoController < ApplicationController def create file = params[:file] - if file.blank? - return redirect_to settings_personalization_path, alert: I18n.t('unable_to_save') - end + return redirect_to settings_personalization_path, alert: I18n.t('unable_to_save') if file.blank? current_account.logo.attach(file) diff --git a/app/controllers/api/templates_documents_controller.rb b/app/controllers/api/templates_documents_controller.rb index 3743815c..23125572 100644 --- a/app/controllers/api/templates_documents_controller.rb +++ b/app/controllers/api/templates_documents_controller.rb @@ -47,7 +47,7 @@ module Api end def replace_document(doc_params) - position = doc_params[:position]&.to_i || 0 + position = doc_params[:position].to_i file = Api::DecodeDocumentFile.call(doc_params[:file], name: doc_params[:name]) documents, = Templates::CreateAttachments.call(@template, { files: [file] }, extract_fields: true) @@ -59,7 +59,8 @@ module Api new_schema = { 'attachment_uuid' => document.uuid, 'name' => document.filename.base } if old_schema - new_doc_has_fields = @template.fields.any? { |f| f['areas']&.any? { |a| a['attachment_uuid'] == document.uuid } } + new_doc_has_fields = + @template.fields.any? { |f| f['areas']&.any? { |a| a['attachment_uuid'] == document.uuid } } unless new_doc_has_fields @template.fields.each do |field| diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb index 9a215c51..a6cd5b84 100644 --- a/app/controllers/teams_controller.rb +++ b/app/controllers/teams_controller.rb @@ -41,7 +41,7 @@ class TeamsController < ApplicationController @teams = current_account.teams.active .left_joins(:users) .where(users: { archived_at: nil }) - .or(current_account.teams.active.left_joins(:users).where(users: { id: nil })) + .or(current_account.teams.active.where.missing(:users)) .select('teams.*, COUNT(users.id) AS active_users_count') .group('teams.id') .order(:name) diff --git a/app/controllers/templates_clone_controller.rb b/app/controllers/templates_clone_controller.rb index 2791f66e..3ce25f91 100644 --- a/app/controllers/templates_clone_controller.rb +++ b/app/controllers/templates_clone_controller.rb @@ -9,7 +9,7 @@ class TemplatesCloneController < ApplicationController @template = Template.new(name: "#{@base_template.name} (#{I18n.t('clone')})") end - def create + def create # rubocop:disable Metrics/AbcSize ActiveRecord::Associations::Preloader.new( records: [@base_template], associations: [{ schema_documents: :preview_images_attachments }] diff --git a/app/jobs/process_submitter_reminders_job.rb b/app/jobs/process_submitter_reminders_job.rb index 16c8b758..681f412c 100644 --- a/app/jobs/process_submitter_reminders_job.rb +++ b/app/jobs/process_submitter_reminders_job.rb @@ -81,7 +81,7 @@ class ProcessSubmitterRemindersJob result end - def duration_to_seconds(key) + def duration_to_seconds(key) # rubocop:disable Metrics/CyclomaticComplexity case key when 'one_hour' then 1.hour when 'two_hours' then 2.hours diff --git a/app/jobs/send_submitter_reminder_email_job.rb b/app/jobs/send_submitter_reminder_email_job.rb index fd402fe0..85949c05 100644 --- a/app/jobs/send_submitter_reminder_email_job.rb +++ b/app/jobs/send_submitter_reminder_email_job.rb @@ -15,7 +15,7 @@ class SendSubmitterReminderEmailJob return unless submitter.email.to_s.include?('@') return unless Accounts.can_send_emails?(submitter.account) return if submitter.submission_events.where(event_type: 'send_reminder_email') - .where('created_at > ?', 1.minute.ago).exists? + .exists?(['created_at > ?', 1.minute.ago]) mail = SubmitterMailer.reminder_email(submitter) diff --git a/app/models/account_config.rb b/app/models/account_config.rb index bff2c61d..895e9a94 100644 --- a/app/models/account_config.rb +++ b/app/models/account_config.rb @@ -62,10 +62,22 @@ class AccountConfig < ApplicationRecord ENABLE_MCP_KEY = 'enable_mcp' EMAIL_VARIABLES = { - SUBMITTER_INVITATION_EMAIL_KEY => %w[template.name submitter.link account.name sender.name sender.first_name sender.email submitter.name submitter.first_name submitter.email].freeze, - SUBMITTER_COMPLETED_EMAIL_KEY => %w[template.name submission.submitters submission.link sender.name sender.first_name sender.email submitter.name submitter.first_name submitter.email].freeze, - SUBMITTER_INVITATION_REMINDER_EMAIL_KEY => %w[template.name submitter.link account.name sender.name sender.first_name sender.email submitter.name submitter.first_name submitter.email].freeze, - SUBMITTER_DOCUMENTS_COPY_EMAIL_KEY => %w[template.name documents.link account.name sender.name sender.first_name sender.email submitter.name submitter.first_name submitter.email].freeze + SUBMITTER_INVITATION_EMAIL_KEY => %w[ + template.name submitter.link account.name sender.name + sender.first_name sender.email submitter.name submitter.first_name submitter.email + ].freeze, + SUBMITTER_COMPLETED_EMAIL_KEY => %w[ + template.name submission.submitters submission.link sender.name + sender.first_name sender.email submitter.name submitter.first_name submitter.email + ].freeze, + SUBMITTER_INVITATION_REMINDER_EMAIL_KEY => %w[ + template.name submitter.link account.name sender.name + sender.first_name sender.email submitter.name submitter.first_name submitter.email + ].freeze, + SUBMITTER_DOCUMENTS_COPY_EMAIL_KEY => %w[ + template.name documents.link account.name sender.name + sender.first_name sender.email submitter.name submitter.first_name submitter.email + ].freeze }.freeze DEFAULT_VALUES = { diff --git a/app/views/notifications_settings/_reminder_queue.html.erb b/app/views/notifications_settings/_reminder_queue.html.erb index 66a5bc09..56e62e46 100644 --- a/app/views/notifications_settings/_reminder_queue.html.erb +++ b/app/views/notifications_settings/_reminder_queue.html.erb @@ -42,9 +42,9 @@ <%= button_to t('skip'), settings_submitter_reminder_path(entry[:submitter]), - method: :delete, - class: 'btn btn-xs btn-outline', - data: { turbo_frame: "reminder_row_#{entry[:submitter].id}" } %> + method: :delete, + class: 'btn btn-xs btn-outline', + data: { turbo_frame: "reminder_row_#{entry[:submitter].id}" } %> <% end %> diff --git a/app/views/personalization_settings/_logo_form.html.erb b/app/views/personalization_settings/_logo_form.html.erb index 3cbca675..d6dcc10d 100644 --- a/app/views/personalization_settings/_logo_form.html.erb +++ b/app/views/personalization_settings/_logo_form.html.erb @@ -1,7 +1,7 @@ <% if current_account.logo.attached? %>
- + <%= button_to t('remove'), settings_account_logo_path, method: :delete, class: 'btn btn-sm btn-error btn-outline', data: { turbo_confirm: t('are_you_sure_') } %>
diff --git a/app/views/shared/_navbar_buttons.html.erb b/app/views/shared/_navbar_buttons.html.erb index 1fbdf5cc..8ec76ba2 100644 --- a/app/views/shared/_navbar_buttons.html.erb +++ b/app/views/shared/_navbar_buttons.html.erb @@ -1,7 +1,7 @@ <% if signed_in? && current_user != true_user %> <%= render 'shared/test_alert' %> <% elsif request.path.starts_with?('/settings') %> - <%= link_to "https://www.docuseal.com", class: 'hidden md:inline-flex btn btn-warning btn-sm', target: '_blank', data: { prefetch: false } do %> + <%= link_to 'https://www.docuseal.com', class: 'hidden md:inline-flex btn btn-warning btn-sm', target: '_blank', rel: 'noopener', data: { prefetch: false } do %> Go Full Pro <% end %> diff --git a/app/views/start_form/_docuseal_logo.html.erb b/app/views/start_form/_docuseal_logo.html.erb index 9ca297c5..22a0ac3f 100644 --- a/app/views/start_form/_docuseal_logo.html.erb +++ b/app/views/start_form/_docuseal_logo.html.erb @@ -1,6 +1,6 @@ <% if @template&.account&.logo&.attached? %> - + <% else %> <%= render 'shared/logo', width: '50px', height: '50px' %> diff --git a/app/views/submissions/_logo.html.erb b/app/views/submissions/_logo.html.erb index d93caf19..d9faccc8 100644 --- a/app/views/submissions/_logo.html.erb +++ b/app/views/submissions/_logo.html.erb @@ -1,5 +1,5 @@ <% if @submission&.account&.logo&.attached? %> - + <% else %> <%= render 'shared/logo', width: 40, height: 40 %> <% end %> diff --git a/app/views/submit_form/_docuseal_logo.html.erb b/app/views/submit_form/_docuseal_logo.html.erb index 1c5e7549..0326577f 100644 --- a/app/views/submit_form/_docuseal_logo.html.erb +++ b/app/views/submit_form/_docuseal_logo.html.erb @@ -1,6 +1,6 @@ <% if @submitter&.account&.logo&.attached? %> - + <% else %> <%= render 'shared/logo', class: 'w-9 h-9 md:w-12 md:h-12' %> <%= Docuseal.product_name %> diff --git a/app/views/submitter_mailer/_custom_content.html.erb b/app/views/submitter_mailer/_custom_content.html.erb index 655f2b1d..c9b02c88 100644 --- a/app/views/submitter_mailer/_custom_content.html.erb +++ b/app/views/submitter_mailer/_custom_content.html.erb @@ -4,8 +4,8 @@ <% if submitter_url_pattern && rendered_html.include?(submitter_url_pattern) %> <% button_label = I18n.t(submitter.with_signature_fields? ? :review_and_sign : :review_and_submit) %> <% rendered_html = rendered_html.gsub(%r{[^<]*}i) do - url = Regexp.last_match(1) - render(partial: 'shared/email_button', locals: { url: url, label: button_label }) - end %> + url = Regexp.last_match(1) + render(partial: 'shared/email_button', locals: { url: url, label: button_label }) + end %> <% end %> <%= rendered_html.html_safe %> diff --git a/app/views/users/_role_select.html.erb b/app/views/users/_role_select.html.erb index 9fb56f69..5d233895 100644 --- a/app/views/users/_role_select.html.erb +++ b/app/views/users/_role_select.html.erb @@ -1,4 +1,4 @@
<%= f.label :role, class: 'label' %> - <%= f.select :role, [['Admin', 'admin'], ['Editor', 'editor']], { selected: f.object.role }, class: 'base-select' %> + <%= f.select :role, [%w[Admin admin], %w[Editor editor]], { selected: f.object.role }, class: 'base-select' %>
diff --git a/bin/ci b/bin/ci new file mode 100755 index 00000000..e37c87b1 --- /dev/null +++ b/bin/ci @@ -0,0 +1,31 @@ +#!/bin/sh +# Run the full CI suite locally via Docker (mirrors GitHub Actions). +# Usage: bin/ci [service] +# bin/ci — run lint + brakeman + rspec +# bin/ci lint — run only linters +# bin/ci rspec — run only tests +# bin/ci brakeman — run only security scanner +set -e + +SERVICE="${1:-}" + +echo "Building CI image (cached)..." +docker compose -f docker-compose.ci.yml build --quiet + +if [ -z "$SERVICE" ]; then + echo "━━━ Lint ━━━" + docker compose -f docker-compose.ci.yml run --rm --no-deps lint + echo "" + echo "━━━ Brakeman ━━━" + docker compose -f docker-compose.ci.yml run --rm --no-deps brakeman + echo "" + echo "━━━ RSpec ━━━" + docker compose -f docker-compose.ci.yml run --rm rspec +else + docker compose -f docker-compose.ci.yml run --rm "$SERVICE" +fi + +docker compose -f docker-compose.ci.yml down --volumes --remove-orphans 2>/dev/null + +echo "" +echo "✅ CI passed." diff --git a/bin/lint b/bin/lint new file mode 100755 index 00000000..59c426fe --- /dev/null +++ b/bin/lint @@ -0,0 +1,12 @@ +#!/bin/sh +# Run all linters via Docker (same as CI pipeline). +# Usage: bin/lint +set -e + +echo "Building CI image (cached)..." +docker compose -f docker-compose.ci.yml build lint --quiet + +echo "Running Rubocop + ERBLint + ESLint..." +docker compose -f docker-compose.ci.yml run --rm --no-deps lint + +echo "✅ All checks passed." diff --git a/db/migrate/20260508100000_create_teams.rb b/db/migrate/20260508100000_create_teams.rb index 008f309d..1ea93571 100644 --- a/db/migrate/20260508100000_create_teams.rb +++ b/db/migrate/20260508100000_create_teams.rb @@ -12,6 +12,6 @@ class CreateTeams < ActiveRecord::Migration[8.0] end add_index :teams, :uuid, unique: true - add_index :teams, %i[account_id name], unique: true, where: "archived_at IS NULL" + add_index :teams, %i[account_id name], unique: true, where: 'archived_at IS NULL' end end diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml new file mode 100644 index 00000000..b8537629 --- /dev/null +++ b/docker-compose.ci.yml @@ -0,0 +1,61 @@ +services: + lint: + build: + context: . + dockerfile: Dockerfile.ci + command: sh -c "bundle exec rubocop && bundle exec erb_lint ./app && yarn eslint 'app/javascript/**/*.js'" + volumes: + - .:/app:ro + - bundle_cache:/usr/local/bundle + - node_cache:/app/node_modules + tmpfs: + - /tmp + + brakeman: + build: + context: . + dockerfile: Dockerfile.ci + command: bundle exec brakeman -q --exit-on-warn + volumes: + - .:/app:ro + - bundle_cache:/usr/local/bundle + - node_cache:/app/node_modules + tmpfs: + - /tmp + + rspec: + build: + context: . + dockerfile: Dockerfile.ci + command: sh -c "bundle exec rake db:create db:migrate && bundle exec rake assets:precompile && bundle exec rspec" + depends_on: + postgres: + condition: service_healthy + environment: + RAILS_ENV: test + NODE_ENV: test + DATABASE_URL: postgres://postgres:postgres@postgres:5432/docuseal_test + volumes: + - .:/app + - bundle_cache:/usr/local/bundle + - node_cache:/app/node_modules + tmpfs: + - /tmp + + postgres: + image: postgres:18 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: postgres + POSTGRES_DB: docuseal_test + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres"] + interval: 5s + timeout: 5s + retries: 5 + tmpfs: + - /var/lib/postgresql/data + +volumes: + bundle_cache: + node_cache: diff --git a/docker-compose.yml b/docker-compose.yml index 920f9ee7..db5247ce 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -3,9 +3,7 @@ services: depends_on: postgres: condition: service_healthy - build: - context: . - dockerfile: Dockerfile + image: ghcr.io/s256/docuseal-with-some-pro-features:latest ports: - 3000:3000 volumes: diff --git a/lib/submitter_reminders.rb b/lib/submitter_reminders.rb index 1ee4eff0..1bf97bf4 100644 --- a/lib/submitter_reminders.rb +++ b/lib/submitter_reminders.rb @@ -3,6 +3,7 @@ module SubmitterReminders module_function + # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def next_reminder_at(submitter, reminder_config) return nil unless reminder_config&.value.is_a?(Hash) return nil if submitter.completed_at? || submitter.declined_at? @@ -35,6 +36,7 @@ module SubmitterReminders base_time + duration end + # rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity def parse_durations(value) return {} unless value.is_a?(Hash) @@ -46,7 +48,7 @@ module SubmitterReminders result end - def duration_to_seconds(key) + def duration_to_seconds(key) # rubocop:disable Metrics/CyclomaticComplexity case key when 'one_hour' then 1.hour when 'two_hours' then 2.hours