refactor(export): reduce repetition in export code hopefully making it more readable

This commit is contained in:
Paul Chavard 2022-11-15 18:53:23 +01:00
parent c1ae9d03be
commit 6f6b3896de
4 changed files with 57 additions and 119 deletions

View file

@ -4,19 +4,14 @@ module Administrateurs
before_action :ensure_not_super_admin! before_action :ensure_not_super_admin!
def download def download
export = Export.find_or_create_export(export_format, all_groupe_instructeurs, **export_options) export = Export.find_or_create_export(export_format, all_groupe_instructeurs, force: force_export?, **export_options)
@dossiers_count = export.count
if export.available? && export.old? && force_export? assign_exports
export.destroy
export = Export.find_or_create_export(export_format, all_groupe_instructeurs, **export_options)
end
if export.available? if export.available?
respond_to do |format| respond_to do |format|
format.turbo_stream do format.turbo_stream do
@dossiers_count = export.count flash.notice = export.flash_message
assign_exports
flash.notice = "Lexport au format \"#{export_format}\" est prêt. Vous pouvez le <a href=\"#{export.file.service_url}\">télécharger</a>"
end end
format.html do format.html do
@ -25,18 +20,13 @@ module Administrateurs
end end
else else
respond_to do |format| respond_to do |format|
notice_message = "Nous générons cet export. Veuillez revenir dans quelques minutes pour le télécharger."
format.turbo_stream do format.turbo_stream do
@dossiers_count = export.count
assign_exports
if !params[:no_progress_notification] if !params[:no_progress_notification]
flash.notice = notice_message flash.notice = export.flash_message
end end
end end
format.html do format.html do
redirect_to admin_procedure_archives_url(@procedure), notice: notice_message redirect_to admin_procedure_archives_url(@procedure), notice: export.flash_message
end end
end end
end end

View file

@ -158,21 +158,17 @@ module Instructeurs
.visible_by_administration .visible_by_administration
.exists?(groupe_instructeur_id: groupe_instructeur_ids) && !instructeur_as_manager? .exists?(groupe_instructeur_id: groupe_instructeur_ids) && !instructeur_as_manager?
export = Export.find_or_create_export(export_format, groupe_instructeurs, **export_options) export = Export.find_or_create_export(export_format, groupe_instructeurs, force: force_export?, **export_options)
if export.available? && export.old? && force_export? @procedure = procedure
export.destroy @statut = export_options[:statut]
export = Export.find_or_create_export(export_format, groupe_instructeurs, **export_options) @dossiers_count = export.count
end assign_exports
if export.available? if export.available?
respond_to do |format| respond_to do |format|
format.turbo_stream do format.turbo_stream do
@procedure = procedure flash.notice = export.flash_message
@statut = export_options[:statut]
@dossiers_count = export.count
assign_exports
flash.notice = "Lexport au format \"#{export_format}\" est prêt. Vous pouvez le <a href=\"#{export.file.service_url}\">télécharger</a>"
end end
format.html do format.html do
@ -181,20 +177,13 @@ module Instructeurs
end end
else else
respond_to do |format| respond_to do |format|
notice_message = "Nous générons cet export. Veuillez revenir dans quelques minutes pour le télécharger."
format.turbo_stream do format.turbo_stream do
@procedure = procedure
@statut = export_options[:statut]
@dossiers_count = export.count
assign_exports
if !params[:no_progress_notification] if !params[:no_progress_notification]
flash.notice = notice_message flash.notice = export.flash_message
end end
end end
format.html do format.html do
redirect_to instructeur_procedure_url(procedure), notice: notice_message redirect_to instructeur_procedure_url(procedure), notice: export.flash_message
end end
end end
end end

View file

@ -83,13 +83,23 @@ class Export < ApplicationRecord
procedure_presentation_id.present? procedure_presentation_id.present?
end end
def self.find_or_create_export(format, groupe_instructeurs, time_span_type: time_span_types.fetch(:everything), statut: statuts.fetch(:tous), procedure_presentation: nil) def flash_message
create_with(groupe_instructeurs: groupe_instructeurs, procedure_presentation: procedure_presentation, procedure_presentation_snapshot: procedure_presentation&.snapshot) if available?
.includes(:procedure_presentation) "Lexport au format \"#{format}\" est prêt. Vous pouvez le <a href=\"#{file.service_url}\">télécharger</a>"
.create_or_find_by(format: format, else
time_span_type: time_span_type, "Nous générons cet export. Veuillez revenir dans quelques minutes pour le télécharger."
statut: statut, end
key: generate_cache_key(groupe_instructeurs.map(&:id), procedure_presentation)) end
def self.find_or_create_export(format, groupe_instructeurs, time_span_type: time_span_types.fetch(:everything), statut: statuts.fetch(:tous), procedure_presentation: nil, force: false)
export = create_or_find_export(format, groupe_instructeurs, time_span_type: time_span_type, statut: statut, procedure_presentation: procedure_presentation)
if export.available? && export.old? && force
export.destroy
create_or_find_export(format, groupe_instructeurs, time_span_type: time_span_type, statut: statut, procedure_presentation: procedure_presentation)
else
export
end
end end
def self.find_for_groupe_instructeurs(groupe_instructeurs_ids, procedure_presentation) def self.find_for_groupe_instructeurs(groupe_instructeurs_ids, procedure_presentation)
@ -121,6 +131,15 @@ class Export < ApplicationRecord
} }
end end
def self.create_or_find_export(format, groupe_instructeurs, time_span_type:, statut:, procedure_presentation:)
create_with(groupe_instructeurs: groupe_instructeurs, procedure_presentation: procedure_presentation, procedure_presentation_snapshot: procedure_presentation&.snapshot)
.includes(:procedure_presentation)
.create_or_find_by(format: format,
time_span_type: time_span_type,
statut: statut,
key: generate_cache_key(groupe_instructeurs.map(&:id), procedure_presentation))
end
def self.generate_cache_key(groupe_instructeurs_ids, procedure_presentation = nil) def self.generate_cache_key(groupe_instructeurs_ids, procedure_presentation = nil)
if procedure_presentation.present? if procedure_presentation.present?
[ [

View file

@ -1,25 +1,5 @@
{ {
"ignored_warnings": [ "ignored_warnings": [
{
"warning_type": "Redirect",
"warning_code": 18,
"fingerprint": "170b506bd3d25eb50f464703f9d993bf2e124d5cbc8478ce9d9d06a15b4bc55e",
"check_name": "Redirect",
"message": "Possible unprotected redirect",
"file": "app/controllers/instructeurs/exports_controller.rb",
"line": 26,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(Export.find_or_create_export(export_format, groupe_instructeurs, **export_options).file.service_url)",
"render_path": null,
"location": {
"type": "method",
"class": "Instructeurs::ExportsController",
"method": "download"
},
"user_input": "Export.find_or_create_export(export_format, groupe_instructeurs, **export_options).file.service_url",
"confidence": "High",
"note": ""
},
{ {
"warning_type": "Cross-Site Scripting", "warning_type": "Cross-Site Scripting",
"warning_code": 2, "warning_code": 2,
@ -35,7 +15,7 @@
"type": "controller", "type": "controller",
"class": "Users::DossiersController", "class": "Users::DossiersController",
"method": "merci", "method": "merci",
"line": 204, "line": 232,
"file": "app/controllers/users/dossiers_controller.rb", "file": "app/controllers/users/dossiers_controller.rb",
"rendered": { "rendered": {
"name": "users/dossiers/merci", "name": "users/dossiers/merci",
@ -92,63 +72,23 @@
"confidence": "Weak", "confidence": "Weak",
"note": "explicitely sanitized even if we are using html_safe" "note": "explicitely sanitized even if we are using html_safe"
}, },
{
"warning_type": "SQL Injection",
"warning_code": 0,
"fingerprint": "4254ed68100af9b496883716b1fd658e1943b2385a0d08de5a6ef5c600c1a8f9",
"check_name": "SQL",
"message": "Possible SQL injection",
"file": "app/models/traitement.rb",
"line": 52,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "ActiveRecord::Base.connection.execute(\"select date_trunc('month', r1.processed_at::TIMESTAMPTZ AT TIME ZONE '#{Time.zone.formatted_offset}'::INTERVAL) as month, count(r1.processed_at)\\nfrom (#{Traitement.select(\"max(traitements.processed_at) as processed_at\").termine.where(:dossier => Dossier.state_termine.where(:groupe_instructeur => groupe_instructeurs)).group(:dossier_id).to_sql}) as r1\\ngroup by date_trunc('month', r1.processed_at::TIMESTAMPTZ AT TIME ZONE '#{Time.zone.formatted_offset}'::INTERVAL)\\norder by month desc\\n\")",
"render_path": null,
"location": {
"type": "method",
"class": "Traitement",
"method": "Traitement.count_dossiers_termines_by_month"
},
"user_input": "Time.zone.formatted_offset",
"confidence": "Medium",
"note": ""
},
{ {
"warning_type": "Redirect", "warning_type": "Redirect",
"warning_code": 18, "warning_code": 18,
"fingerprint": "589e457617e6ad3d35f454b4af8ed326817930d6a724febcf680d7e03c6aeaa2", "fingerprint": "8a1ccc92988486094b2c89e586902a3b6fcbd43910d6363dce14b9981ca8ddeb",
"check_name": "Redirect", "check_name": "Redirect",
"message": "Possible unprotected redirect", "message": "Possible unprotected redirect",
"file": "app/controllers/targeted_user_links_controller.rb", "file": "app/controllers/instructeurs/procedures_controller.rb",
"line": 7, "line": 175,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/", "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(TargetedUserLink.find(params[:id]).redirect_url(Rails.application.routes.url_helpers))", "code": "redirect_to(Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), :force => force_export?, **export_options).file.service_url)",
"render_path": null, "render_path": null,
"location": { "location": {
"type": "method", "type": "method",
"class": "TargetedUserLinksController", "class": "Instructeurs::ProceduresController",
"method": "show" "method": "download_export"
}, },
"user_input": "TargetedUserLink.find(params[:id]).redirect_url(Rails.application.routes.url_helpers)", "user_input": "Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), :force => force_export?, **export_options).file.service_url",
"confidence": "High",
"note": ""
},
{
"warning_type": "Redirect",
"warning_code": 18,
"fingerprint": "8f8133f0679f6301e098c4b9c61c2217224126856963abd02cdc5e54efb4e818",
"check_name": "Redirect",
"message": "Possible unprotected redirect",
"file": "app/controllers/administrateurs/exports_controller.rb",
"line": 22,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(Export.find_or_create_export(export_format, all_groupe_instructeurs, **export_options).file.service_url)",
"render_path": null,
"location": {
"type": "method",
"class": "Administrateurs::ExportsController",
"method": "download"
},
"user_input": "Export.find_or_create_export(export_format, all_groupe_instructeurs, **export_options).file.service_url",
"confidence": "High", "confidence": "High",
"note": "" "note": ""
}, },
@ -159,7 +99,7 @@
"check_name": "SQL", "check_name": "SQL",
"message": "Possible SQL injection", "message": "Possible SQL injection",
"file": "app/models/concerns/dossier_filtering_concern.rb", "file": "app/models/concerns/dossier_filtering_concern.rb",
"line": 25, "line": 30,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "where(\"#{values.count} OR #{\"(#{ProcedurePresentation.sanitized_column(table, column)} ILIKE ?)\"}\", *values.map do\n \"%#{value}%\"\n end)", "code": "where(\"#{values.count} OR #{\"(#{ProcedurePresentation.sanitized_column(table, column)} ILIKE ?)\"}\", *values.map do\n \"%#{value}%\"\n end)",
"render_path": null, "render_path": null,
@ -175,24 +115,24 @@
{ {
"warning_type": "Redirect", "warning_type": "Redirect",
"warning_code": 18, "warning_code": 18,
"fingerprint": "eae88be293b3849e07be81a18de99c04b08c7b2c045cc4a43ca3624ea178d965", "fingerprint": "e2220b7cda7df5d02de77e7c3ce137653126e0d8e91ce445676b63ec4c94bbcb",
"check_name": "Redirect", "check_name": "Redirect",
"message": "Possible unprotected redirect", "message": "Possible unprotected redirect",
"file": "app/controllers/instructeurs/procedures_controller.rb", "file": "app/controllers/administrateurs/exports_controller.rb",
"line": 168, "line": 18,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/", "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), **export_options).file.service_url)", "code": "redirect_to(Export.find_or_create_export(export_format, all_groupe_instructeurs, :force => force_export?, **export_options).file.service_url)",
"render_path": null, "render_path": null,
"location": { "location": {
"type": "method", "type": "method",
"class": "Instructeurs::ProceduresController", "class": "Administrateurs::ExportsController",
"method": "download_export" "method": "download"
}, },
"user_input": "Export.find_or_create_export(export_format, current_instructeur.groupe_instructeurs.where(:procedure => procedure), **export_options).file.service_url", "user_input": "Export.find_or_create_export(export_format, all_groupe_instructeurs, :force => force_export?, **export_options).file.service_url",
"confidence": "High", "confidence": "High",
"note": "" "note": ""
} }
], ],
"updated": "2022-07-08 13:14:19 +0200", "updated": "2022-11-15 23:04:53 +0100",
"brakeman_version": "5.2.2" "brakeman_version": "5.2.2"
} }