Move to grover for PDF generation & add a dedicated download button (#1194)

* Move to grover for PDF generation

* incorporate the AI suggestions

* fix lint

* fix double auth

* fix tests

* fix last issues

* fix errorhandeling

* fix lint

* add puppeteer

* fix zatladder spending being zero

* make table header and footer bold

* fix lint

Author: lodewiges

Dockerfile

@@ -15,7 +15,7 @@ RUN apt-get update -qq && \
   libpq-dev \
   curl \
   netcat-traditional \
-  wkhtmltopdf \
+  chromium \
   libyaml-dev
 
 # Add Node, required for asset pipeline.

Gemfile

@@ -10,6 +10,7 @@ gem 'devise', '~>4.9.4'
 gem 'devise-i18n', '~>1.15.0'
 gem 'factory_bot_rails', '~> 6.5.1'
 gem 'faker', '~> 3.5.3'
+gem 'grover', '~> 1.2.4'
 gem 'http', '~> 5.3.1'
 gem 'jbuilder', '~> 2.14.1'
 gem 'jquery-rails', '~> 4.6.1'
@@ -43,8 +44,6 @@ gem 'sprockets-rails', '~> 3.5', '>= 3.5.2'
 gem 'turbo-rails', '~> 2.0', '>= 2.0.20'
 gem 'uglifier', '~> 4.2.1'
 gem 'validates_timeliness', '~> 7.1.0'
-gem 'wicked_pdf', '~> 2.8.2'
-gem 'wkhtmltopdf-binary', '~> 0.12.6.10'
 
 group :development, :test do
   gem 'awesome_print', '~> 1.9.2'

Gemfile.lock

@@ -183,6 +183,8 @@ GEM
     google-protobuf (4.33.0-x86_64-linux-gnu)
       bigdecimal
       rake (>= 13)
+    grover (1.2.4)
+      nokogiri (~> 1)
     guard (2.19.1)
       formatador (>= 0.2.4)
       listen (>= 2.7, < 4.0)
@@ -596,10 +598,6 @@ GEM
       base64
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
-    wicked_pdf (2.8.2)
-      activesupport
-      ostruct
-    wkhtmltopdf-binary (0.12.6.10)
     zeitwerk (2.7.3)
 
 PLATFORMS
@@ -625,6 +623,7 @@ DEPENDENCIES
   factory_bot_rails (~> 6.5.1)
   faker (~> 3.5.3)
   foreman (~> 0.90.0)
+  grover (~> 1.2.4)
   guard-livereload (~> 2.5.2)
   guard-rspec (~> 4.7.3)
   http (~> 5.3.1)
@@ -682,8 +681,6 @@ DEPENDENCIES
   uglifier (~> 4.2.1)
   validates_timeliness (~> 7.1.0)
   web-console (~> 4.2.1)
-  wicked_pdf (~> 2.8.2)
-  wkhtmltopdf-binary (~> 0.12.6.10)
 
 BUNDLED WITH
    2.6.9

app/assets/stylesheets/pdf.scss

@@ -1,7 +1 @@
-// There is a bug with Bootstrap tables in PDF, so we use td in a th, but it needs to be bold
-thead,
-tfoot {
-  td {
-    font-weight: bold;
-  }
-}
+// PDF-specific styles

app/controllers/invoices_controller.rb

@@ -15,14 +15,14 @@ def index
 
   def show
     @invoice = invoice
+    token_based_access = !integer_id?(params[:id])
+
+    # Authorize for authenticated access (integer ID), skip for token-based access
+    authorize @invoice, :show? unless token_based_access
 
     respond_to do |format|
       format.html
-      format.pdf do
-        render pdf: "Factuur #{@invoice.human_id}",
-               template: 'invoices/show.html.erb',
-               lowquality: true
-      end
+      format.pdf { render_invoice_pdf }
     end
   end
 
@@ -42,6 +42,9 @@ def create
 
   def pay # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
     @invoice = invoice
+    token_based_access = !integer_id?(params[:id])
+
+    authorize @invoice, :pay? unless token_based_access
 
     if @invoice.paid?
       redirect_to invoice_path params[:id]
@@ -73,14 +76,41 @@ def send_invoice
 
   private
 
+  def integer_id?(id)
+    Integer(id)
+    true
+  rescue ArgumentError
+    false
+  end
+
   def invoice
     @invoice = Invoice.find(Integer(params[:id]))
-    authorize @invoice
   rescue ArgumentError
     @invoice = Invoice.find_by!(token: params[:id])
   end
 
   def permitted_attributes
     params.require(:invoice).permit(%i[user_id activity_id name_override email_override rows], rows_attributes: %i[name amount price])
   end
+
+  def render_invoice_pdf # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
+    token_based_access = !integer_id?(params[:id])
+    authorize @invoice, :download? unless token_based_access
+
+    html = render_to_string(
+      template: 'invoices/show',
+      formats: [:html],
+      layout: 'pdf'
+    )
+    pdf = Grover.new(html).to_pdf
+    send_data pdf, filename: "Factuur-#{@invoice.human_id}.pdf", type: 'application/pdf', disposition: 'attachment'
+  rescue StandardError => e
+    Rails.logger.error "Failed to generate PDF for invoice #{@invoice.id}: #{e.message}"
+    if request.format.pdf?
+      render plain: 'Er is een fout opgetreden bij het genereren van de PDF. Probeer het later opnieuw.', status: :internal_server_error
+    else
+      flash[:error] = 'Er is een fout opgetreden bij het genereren van de PDF. Probeer het later opnieuw.'
+      redirect_to invoice_path(@invoice)
+    end
+  end
 end

app/controllers/zatladder_controller.rb

@@ -23,6 +23,7 @@ def current_year
     end
   end
 
+  # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
   def zatladder_spendings(from, to)
     users = User.in_amber.exists? ? User.in_amber : User.sofia_account
     @users_spendings = users.calculate_spendings(from:, to:)
@@ -33,6 +34,8 @@ def zatladder_spendings(from, to)
         spendings: @users_spendings.fetch(user.id, 0)
       }
     end
+    zatladder.reject! { |user| user[:spendings].zero? }
     zatladder.sort_by { |id| id[:spendings] }.reverse!
   end
+  # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
 end

app/mailers/invoice_mailer.rb

@@ -9,9 +9,19 @@ def invoice_mail(invoice) # rubocop:disable Metrics/AbcSize, Metrics/MethodLengt
       @cab_disabled = true
     end
 
-    attachments["#{invoice.human_id}.pdf"] = WickedPdf.new.pdf_from_string(
-      render_to_string(pdf: invoice.human_id.to_s, template: 'invoices/show', layout: 'pdf')
-    )
+    begin
+      html = render_to_string(
+        template: 'invoices/show',
+        formats: [:html],
+        layout: 'pdf'
+      )
+      pdf = Grover.new(html).to_pdf
+      attachments["#{invoice.human_id}.pdf"] = pdf
+    rescue StandardError => e
+      Rails.logger.error "Failed to generate PDF attachment for invoice #{invoice.id}: #{e.message}"
+      Rails.logger.error e.backtrace.join("\n")
+      # Continue sending email without PDF attachment
+    end
 
     mail to: @invoice.email, subject: "Factuur #{invoice.human_id} #{Rails.application.config.x.company_name}"
   end

app/policies/invoice_policy.rb

@@ -7,6 +7,10 @@ def send_invoice?
     user&.treasurer?
   end
 
+  def download?
+    user&.treasurer?
+  end
+
   def pay?
     show?
   end

app/views/invoices/index.html.erb

@@ -35,6 +35,9 @@
             <% if policy(Invoice).send_invoice? %>
               <th scope="col">Verstuur</th>
             <% end %>
+            <% if policy(Invoice).download? %>
+              <th scope="col">Download</th>
+            <% end %>
           </tr>
           </thead>
           <tbody class="table-group-divider">
@@ -77,6 +80,11 @@
                   <% end %>
                 </td>
               <% end %>
+              <% if policy(Invoice).download? %>
+                <td>
+                  <%= link_to 'Downloaden', invoice_path(invoice, format: :pdf), class: 'btn btn-primary', download: "Factuur-#{invoice.human_id}.pdf" %>
+                </td>
+              <% end %>
             </tr>
           <% end %>
           </tbody>

app/views/invoices/show.html.erb

@@ -94,9 +94,9 @@
         <table class="table table-striped">
           <thead>
           <tr>
-            <td scope="col" class="col-8">Consumpties</td>
-            <td scope="col" class="col-2">Aantal</td>
-            <td scope="col" class="col-2">Totaalbedrag</td>
+            <th scope="col" class="col-8">Consumpties</th>
+            <th scope="col" class="col-2">Aantal</th>
+            <th scope="col" class="col-2">Totaalbedrag</th>
           </tr>
           </thead>
           <tbody class="table-group-divider">
@@ -110,9 +110,9 @@
           </tbody>
           <tfoot class="table-group-divider">
             <tr>
-              <td class="border-bottom-0">Totaal</td>
+              <th class="border-bottom-0">Totaal</th>
               <td class="border-bottom-0" />
-              <td class="border-bottom-0"><%= number_to_currency(@invoice.activity.revenue_by_user(@invoice.user), unit: '€') %></td>
+              <th class="border-bottom-0"><%= number_to_currency(@invoice.activity.revenue_by_user(@invoice.user), unit: '€') %></th>
             </tr>
           </tfoot>
         </table>
@@ -124,9 +124,9 @@
         <table class="table table-striped">
           <thead>
           <tr>
-            <td scope="col" class="col-8"><% if @invoice.activity.count_per_product(user: @invoice.user).any? %>Overige kosten<% else %>Kosten<% end %></td>
-            <td scope="col" class="col-2">Aantal</td>
-            <td scope="col" class="col-2">Totaalbedrag</td>
+            <th scope="col" class="col-8"><% if @invoice.activity.count_per_product(user: @invoice.user).any? %>Overige kosten<% else %>Kosten<% end %></th>
+            <th scope="col" class="col-2">Aantal</th>
+            <th scope="col" class="col-2">Totaalbedrag</th>
           </tr>
           </thead>
           <tbody class="table-group-divider">
@@ -140,9 +140,9 @@
           </tbody>
           <tfoot class="table-group-divider">
             <tr>
-              <td class="border-bottom-0">Totaal</td>
+              <th class="border-bottom-0">Totaal</th>
               <td class="border-bottom-0" />
-              <td class="border-bottom-0"><%= number_to_currency(@invoice.row_amount, unit: '€') %></td>
+              <th class="border-bottom-0"><%= number_to_currency(@invoice.row_amount, unit: '€') %></th>
             </tr>
           </tfoot>
         </table>