Mark timesheet entries as billed only when invoice is sent succes…

…sfully (#1860)

* Mark timesheet entries as `billed` only when invoice is `sent` successfully

- Fix #1853
- Couple of weeks back, invoices stuck in `sending` state due to insufficient memory at sidekiq
- And, we found this issue that timesheet entries were `billed`
- These timesheet entries must be marked as `billed` only when invoice
  is in `sent` state

* Fix failing spec

---------

Co-authored-by: Nishant Samel <nishant@saeloun.com>

app/controllers/internal_api/v1/invoices_controller.rb

@@ -2,7 +2,6 @@
 
 class InternalApi::V1::InvoicesController < InternalApi::V1::ApplicationController
   before_action :load_client, only: [:create, :update]
-  after_action :ensure_time_entries_billed, only: [:send_invoice]
   after_action :track_event, only: [:create, :update, :destroy, :send_invoice, :download]
 
   def index
@@ -122,10 +121,6 @@ def invoice_email_params
       params.require(:invoice_email).permit(:subject, :message, recipients: [])
     end
 
-    def ensure_time_entries_billed
-      invoice.update_timesheet_entry_status!
-    end
-
     def track_event
       Invoices::EventTrackerService.new(params[:action], @invoice || invoice, params).process
     end

app/mailers/invoice_mailer.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class InvoiceMailer < ApplicationMailer
-  after_action -> { @invoice.sent! if @invoice.draft? || @invoice.viewed? || @invoice.declined? || @invoice.sending? }
+  after_action :update_status, only: [:invoice]
 
   def invoice
     @invoice = Invoice.find(params[:invoice_id])
@@ -37,4 +37,11 @@ def company_logo
     def can_send_invoice?
       @invoice.sending? && @invoice.recently_sent_mail?
     end
+
+    def update_status
+      if @invoice.draft? || @invoice.viewed? || @invoice.declined? || @invoice.sending?
+        @invoice.sent!
+        @invoice.update_timesheet_entry_status!
+      end
+    end
 end

spec/requests/internal_api/v1/invoices/send_invoice_spec.rb

@@ -29,24 +29,31 @@
         sign_in user
       end
 
-      # it "returns a 202 response" do
-      #   post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
-      #     headers: auth_headers(user)
+      it "returns a 202 response" do
+        post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
+          headers: auth_headers(user)
 
-      #   expect(response).to have_http_status :accepted
-      #   expect(json_response["message"]).to eq("Invoice will be sent!")
-      # end
+        expect(response).to have_http_status :accepted
+        expect(json_response["message"]).to eq("Invoice will be sent!")
+      end
 
-      # it "enqueues an email for delivery" do
-      #   expect do
-      #     post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
-      #       headers: auth_headers(user)
-      #   end.to have_enqueued_mail(InvoiceMailer, :invoice)
-      # end
+      it "enqueues an email for delivery" do
+        expect do
+          post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
+            headers: auth_headers(user)
+        end.to have_enqueued_mail(InvoiceMailer, :invoice)
+      end
 
       it "changes time_sheet_entries status to billed" do
-        post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
-          headers: auth_headers(user)
+        expect do
+          post send_invoice_internal_api_v1_invoice_path(id: invoice.id), params: { invoice_email: },
+            headers: auth_headers(user)
+        end.to have_enqueued_mail(InvoiceMailer, :invoice)
+
+        perform_enqueued_jobs do
+          InvoiceMailer.with({ invoice_id: invoice.id }.merge(invoice_email)).invoice.deliver_later
+        end
+
         invoice.invoice_line_items.reload.each do |line_item|
           expect(line_item.timesheet_entry.bill_status).to eq("billed")
         end