From 68b112e00a3a481cd5ff6e7b02c50c62b48b5a54 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 6 Jan 2022 12:43:02 +0100 Subject: [PATCH 01/17] helpers: don't crash on unknown flash level Errors generated by the `invisible-catcha` gem are reported as `flash[:error]` (which differs from Rail's usual `flash[:alert]`). Avoid crashing when the flash level passed to this helper is not known. --- app/helpers/application_helper.rb | 9 ++++++--- spec/helpers/application_helper_spec.rb | 6 ++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 98ec37d23..c4dbcee21 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -8,12 +8,15 @@ module ApplicationHelper end def flash_class(level, sticky: false, fixed: false) - class_names = case level + class_names = [] + + case level when 'notice' - ['alert-success'] + class_names << 'alert-success' when 'alert' - ['alert-danger'] + class_names << 'alert-danger' end + if sticky class_names << 'sticky' end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index ff70654fd..cb7f12e70 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -18,6 +18,12 @@ describe ApplicationHelper do end end + describe "#flash_class" do + it { expect(flash_class('notice')).to eq 'alert-success' } + it { expect(flash_class('alert', sticky: true, fixed: true)).to eq 'alert-danger sticky alert-fixed' } + it { expect(flash_class('unknown-level')).to eq '' } + end + describe "#try_format_date" do subject { try_format_date(date) } From 2e1a3c32cb88480db4603012f00acb733ca147e8 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 6 Jan 2022 13:21:47 +0100 Subject: [PATCH 02/17] helpers: handle flash[:error] Errors generated by the `invisible-catcha` gem are reported as `flash[:error]` (which differs from Rail's usual `flash[:alert]`). We probably shouldn't use flash[:error] in our own codebase; but we can handle its use in third-party libraries. --- app/helpers/application_helper.rb | 2 +- spec/helpers/application_helper_spec.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index c4dbcee21..e5ee529a3 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -13,7 +13,7 @@ module ApplicationHelper case level when 'notice' class_names << 'alert-success' - when 'alert' + when 'alert', 'error' class_names << 'alert-danger' end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index cb7f12e70..7fd93a96c 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -21,6 +21,7 @@ describe ApplicationHelper do describe "#flash_class" do it { expect(flash_class('notice')).to eq 'alert-success' } it { expect(flash_class('alert', sticky: true, fixed: true)).to eq 'alert-danger sticky alert-fixed' } + it { expect(flash_class('error')).to eq 'alert-danger' } it { expect(flash_class('unknown-level')).to eq '' } end From 383a92bcd9ed3c9b73831069de6f2cc7a03c28a7 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 6 Jan 2022 15:42:56 +0100 Subject: [PATCH 03/17] fix(parallel_download_queue): tophoeus does not like raise from a request handler [crash straight to first frame] --- .../parallel_download_queue.rb | 15 +- .../procedure_attachments_export.rb | 18 +- app/lib/utils/retryable.rb | 20 --- .../cassettes/archive/file_to_get.yml | 51 ------ .../archive/file_to_get_typhoeus.yml | 145 ---------------- .../cassettes/archive/new_file_to_get_200.yml | 80 +++++++++ .../archive/new_file_to_get_400_html.yml | 105 ++++++++++++ .../cassettes/archive/old_file_to_get_200.yml | 157 ++++++++++++++++++ spec/lib/utils/retryable_spec.rb | 36 ---- .../procedure_archive_service_spec.rb | 35 +++- 10 files changed, 383 insertions(+), 279 deletions(-) delete mode 100644 app/lib/utils/retryable.rb delete mode 100644 spec/fixtures/cassettes/archive/file_to_get.yml delete mode 100644 spec/fixtures/cassettes/archive/file_to_get_typhoeus.yml create mode 100644 spec/fixtures/cassettes/archive/new_file_to_get_200.yml create mode 100644 spec/fixtures/cassettes/archive/new_file_to_get_400_html.yml create mode 100644 spec/fixtures/cassettes/archive/old_file_to_get_200.yml delete mode 100644 spec/lib/utils/retryable_spec.rb diff --git a/app/lib/download_manager/parallel_download_queue.rb b/app/lib/download_manager/parallel_download_queue.rb index ab734e30d..f7c693200 100644 --- a/app/lib/download_manager/parallel_download_queue.rb +++ b/app/lib/download_manager/parallel_download_queue.rb @@ -1,6 +1,5 @@ module DownloadManager class ParallelDownloadQueue - include Utils::Retryable DOWNLOAD_MAX_PARALLEL = ENV.fetch('DOWNLOAD_MAX_PARALLEL') { 10 } attr_accessor :attachments, @@ -17,11 +16,9 @@ module DownloadManager attachments.map do |attachment, path| begin - with_retry(max_attempt: 1) do - download_one(attachment: attachment, - path_in_download_dir: path, - http_client: hydra) - end + download_one(attachment: attachment, + path_in_download_dir: path, + http_client: hydra) rescue => e on_error.call(attachment, path, e) end @@ -47,14 +44,12 @@ module DownloadManager request.on_complete do |response| fd.close unless response.success? - raise 'ko' + File.delete(attachment_path) if File.exist?(attachment_path) # -> case of retries failed, must cleanup partialy downloaded file + on_error.call(attachment, path_in_download_dir, response.code) end end http_client.queue(request) end - rescue - File.delete(attachment_path) if File.exist?(attachment_path) # -> case of retries failed, must cleanup partialy downloaded file - raise end # rubocop:enable Style/AutoResourceCleanup end diff --git a/app/lib/download_manager/procedure_attachments_export.rb b/app/lib/download_manager/procedure_attachments_export.rb index d343c343d..bae43a39a 100644 --- a/app/lib/download_manager/procedure_attachments_export.rb +++ b/app/lib/download_manager/procedure_attachments_export.rb @@ -9,18 +9,20 @@ module DownloadManager @procedure = procedure @errors = {} @queue = ParallelDownloadQueue.new(attachments, destination) - @queue.on_error = proc do |_attachment, path, error| - errors[path] = true + @queue.on_error = proc do |attachment, path, error| + errors[path] = [attachment, path] Rails.logger.error("Fail to download filename #{path} in procedure##{@procedure.id}, reason: #{error}") end - end - - def download_all - @queue.download_all - write_report if !errors.empty? end - private + def download_all(attempt_left: 1) + @queue.download_all + if !errors.empty? && attempt_left.positive? + retryable_queue = self.class.new(@procedure, errors.values, destination) + retryable_queue.download_all(attempt_left: 0) + retryable_queue.write_report if !retryable_queue.errors.empty? + end + end def write_report manifest_path = File.join(destination, 'LISEZMOI.txt') diff --git a/app/lib/utils/retryable.rb b/app/lib/utils/retryable.rb deleted file mode 100644 index d4ab13fca..000000000 --- a/app/lib/utils/retryable.rb +++ /dev/null @@ -1,20 +0,0 @@ -module Utils - module Retryable - # usage: - # max_attempt : retry count - # errors : only retry those errors - # with_retry(max_attempt: 10, errors: [StandardError]) do - # do_something_which_can_fail - # end - def with_retry(max_attempt: 1, errors: [StandardError], &block) - limiter = 0 - begin - yield - rescue *errors - limiter += 1 - retry if limiter <= max_attempt - raise - end - end - end -end diff --git a/spec/fixtures/cassettes/archive/file_to_get.yml b/spec/fixtures/cassettes/archive/file_to_get.yml deleted file mode 100644 index 3e4f18e24..000000000 --- a/spec/fixtures/cassettes/archive/file_to_get.yml +++ /dev/null @@ -1,51 +0,0 @@ ---- -http_interactions: -- request: - method: get - uri: http://file.to/get.ext - body: - encoding: US-ASCII - string: '' - headers: - User-Agent: - - demarches-simplifiees.fr - Expect: - - '' - response: - status: - code: 200 - message: '' - headers: - Last-Modified: - - Thu, 16 Dec 2021 13:04:07 GMT - X-Trans-Id: - - tx62bf43b03d7e4b60b3f25-0061d45dbd - Accept-Ranges: - - bytes - Expires: - - Tue, 04 Jan 2022 15:20:54 GMT - X-Openstack-Request-Id: - - tx62bf43b03d7e4b60b3f25-0061d45dbd - Content-Type: - - image/png - Date: - - Tue, 04 Jan 2022 14:46:21 GMT - X-Iplb-Request-Id: - - 877D6D0C:B8E4_5762BBC9:01BB_61D45DBD_104936A5:293F4 - X-Timestamp: - - '1639659846.52947' - Etag: - - 49961feab1c277af65fcb876c379cebf - X-Iplb-Instance: - - '42085' - Content-Length: - - '494761' - Content-Disposition: - - inline; filename="Screen Shot 2021-12-09 at 9.42.44 AM.png"; filename*=UTF-8''Screen%20Shot%202021-12-09%20at%209.42.44%20AM.png - Strict-Transport-Security: - - max-age=63072000 - body: - encoding: ASCII-8BIT - string: '' - recorded_at: Wed, 04 Mar 2020 23:00:00 GMT -recorded_with: VCR 6.0.0 diff --git a/spec/fixtures/cassettes/archive/file_to_get_typhoeus.yml b/spec/fixtures/cassettes/archive/file_to_get_typhoeus.yml deleted file mode 100644 index 557f10345..000000000 --- a/spec/fixtures/cassettes/archive/file_to_get_typhoeus.yml +++ /dev/null @@ -1,145 +0,0 @@ ---- -http_interactions: -- request: - method: get - uri: https://i.etsystatic.com/6212702/r/il/744d2c/470726480/il_1588xN.470726480_bpk5.jpg - body: - encoding: US-ASCII - string: '' - headers: - User-Agent: - - demarches-simplifiees.fr - Expect: - - '' - response: - status: - code: 200 - message: '' - headers: - Cache-Control: - - public, max-age=365000000, immutable - Content-Type: - - image/jpeg - Etag: - - '"J9Qt3QnUKZIbmKehfH+pmv/rYxafyM81ENfBKsCN6Qw"' - Expires: - - Mon, 02 Jan 2023 05:24:30 GMT - Fastly-Io-Info: - - ifsz=14677 idim=600x600 ifmt=jpeg ofsz=43339 odim=1588x1588 ofmt=jpeg - Fastly-Stats: - - io=1 - Server: - - UploadServer - X-Goog-Generation: - - '1514228438620441' - X-Goog-Hash: - - crc32c=ZohETA== - - md5=iKPGsaOoUN0hhNZYYzYDLQ== - X-Goog-Metageneration: - - '1' - X-Goog-Storage-Class: - - MULTI_REGIONAL - X-Goog-Stored-Content-Encoding: - - identity - X-Goog-Stored-Content-Length: - - '14677' - X-Guploader-Uploadid: - - ADPycdvXmKF1KUStMVeN1v5TUKBQA_YezSueBDRwp4qiVKTn5IDoW7f3_t6_tyvJwjkOoUE4lO1cC_NxSl-LkM5ukthJcv6JkA - Via: - - 1.1 varnish, 1.1 varnish - Accept-Ranges: - - bytes - Date: - - Tue, 04 Jan 2022 14:59:40 GMT - Age: - - '207310' - X-Served-By: - - cache-mdw17340-MDW, cache-cdg20755-CDG - X-Cache: - - HIT, HIT - X-Cache-Hits: - - 1, 1 - X-Timer: - - S1641308380.238785,VS0,VE1 - Vary: - - Accept - Strict-Transport-Security: - - max-age=300 - Content-Length: - - '43339' - body: - encoding: ASCII-8BIT - string: '' - recorded_at: Wed, 04 Mar 2020 23:00:00 GMT -- request: - method: get - uri: https://i.etsystatic.com/6212702/r/il/744d2c/470726480/il_1588xN.470726480_bpk5.jpg - body: - encoding: US-ASCII - string: '' - headers: - User-Agent: - - demarches-simplifiees.fr - Expect: - - '' - response: - status: - code: 200 - message: '' - headers: - Cache-Control: - - public, max-age=365000000, immutable - Content-Type: - - image/jpeg - Etag: - - '"J9Qt3QnUKZIbmKehfH+pmv/rYxafyM81ENfBKsCN6Qw"' - Expires: - - Mon, 02 Jan 2023 05:24:30 GMT - Fastly-Io-Info: - - ifsz=14677 idim=600x600 ifmt=jpeg ofsz=43339 odim=1588x1588 ofmt=jpeg - Fastly-Stats: - - io=1 - Server: - - UploadServer - X-Goog-Generation: - - '1514228438620441' - X-Goog-Hash: - - crc32c=ZohETA== - - md5=iKPGsaOoUN0hhNZYYzYDLQ== - X-Goog-Metageneration: - - '1' - X-Goog-Storage-Class: - - MULTI_REGIONAL - X-Goog-Stored-Content-Encoding: - - identity - X-Goog-Stored-Content-Length: - - '14677' - X-Guploader-Uploadid: - - ADPycdvXmKF1KUStMVeN1v5TUKBQA_YezSueBDRwp4qiVKTn5IDoW7f3_t6_tyvJwjkOoUE4lO1cC_NxSl-LkM5ukthJcv6JkA - Via: - - 1.1 varnish, 1.1 varnish - Accept-Ranges: - - bytes - Date: - - Tue, 04 Jan 2022 14:59:40 GMT - Age: - - '207310' - X-Served-By: - - cache-mdw17340-MDW, cache-cdg20737-CDG - X-Cache: - - HIT, HIT - X-Cache-Hits: - - 1, 1 - X-Timer: - - S1641308380.241689,VS0,VE1 - Vary: - - Accept - Strict-Transport-Security: - - max-age=300 - Content-Length: - - '43339' - body: - encoding: ASCII-8BIT - string: '' - recorded_at: Wed, 04 Mar 2020 23:00:00 GMT -recorded_with: VCR 6.0.0 diff --git a/spec/fixtures/cassettes/archive/new_file_to_get_200.yml b/spec/fixtures/cassettes/archive/new_file_to_get_200.yml new file mode 100644 index 000000000..b8e968d95 --- /dev/null +++ b/spec/fixtures/cassettes/archive/new_file_to_get_200.yml @@ -0,0 +1,80 @@ +--- +http_interactions: +- request: + method: get + uri: https://opengraph.githubassets.com/d0e7862b24d8026a3c03516d865b28151eb3859029c6c6c2e86605891fbdcd7a/socketry/async-io + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 200 + message: '' + headers: + X-Ratelimit-Limit: + - '100' + X-Ratelimit-Remaining: + - '31' + X-Ratelimit-Reset: + - '1641406997' + Access-Control-Allow-Origin: + - "*" + Content-Security-Policy: + - default-src 'none';style-src 'unsafe-inline';font-src https://github.github.com;img-src + https://avatars.githubusercontent.com https://github.githubassets.com https://camo.githubusercontent.com + X-Dns-Prefetch-Control: + - 'off' + Expect-Ct: + - max-age=0 + X-Frame-Options: + - SAMEORIGIN + X-Download-Options: + - noopen + X-Content-Type-Options: + - nosniff + X-Permitted-Cross-Domain-Policies: + - none + Referrer-Policy: + - no-referrer + X-Xss-Protection: + - '0' + Cache-Control: + - public, max-age=21600, immutable + Content-Type: + - image/png + Etag: + - W/"106ec-qZkXmv4Ygfd8LZzQoW8mwToCU7k" + X-Github-Backend: + - Kubernetes + X-Github-Request-Id: + - 2BDC:1C4E:5B580:368C45:61D5E1B1 + Via: + - 1.1 varnish, 1.1 varnish + Accept-Ranges: + - bytes + Date: + - Thu, 06 Jan 2022 14:38:48 GMT + Age: + - '3435' + X-Served-By: + - cache-iad-kiad7000127-IAD, cache-cdg20776-CDG + X-Cache: + - HIT, HIT + X-Cache-Hits: + - 1, 1 + Strict-Transport-Security: + - max-age=31536000 + X-Fastly-Request-Id: + - '064738cfe513e9a05fc3af47b513b4f63b3f199c' + Content-Length: + - '67308' + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +recorded_with: VCR 6.0.0 diff --git a/spec/fixtures/cassettes/archive/new_file_to_get_400_html.yml b/spec/fixtures/cassettes/archive/new_file_to_get_400_html.yml new file mode 100644 index 000000000..91bb6092c --- /dev/null +++ b/spec/fixtures/cassettes/archive/new_file_to_get_400_html.yml @@ -0,0 +1,105 @@ +--- +http_interactions: +- request: + method: get + uri: https://www.demarches-simplifiees.fr/error_2 + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 404 + message: '' + headers: + Server: + - nginx + Date: + - Thu, 06 Jan 2022 14:39:31 GMT + Content-Type: + - text/html; charset=UTF-8 + Content-Length: + - '1583' + X-Request-Id: + - 8881aea9-7dfc-442b-b818-a2988a8a39ee + X-Runtime: + - '0.003713' + Strict-Transport-Security: + - max-age=63072000 + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +- request: + method: get + uri: https://www.demarches-simplifiees.fr/error_1 + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 404 + message: '' + headers: + Server: + - nginx + Date: + - Thu, 06 Jan 2022 14:53:09 GMT + Content-Type: + - text/html; charset=UTF-8 + Content-Length: + - '1583' + X-Request-Id: + - ea764501-134e-49a4-bd65-7fb96f772908 + X-Runtime: + - '0.005202' + Strict-Transport-Security: + - max-age=63072000 + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +- request: + method: get + uri: https://www.demarches-simplifiees.fr/error_1 + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 404 + message: '' + headers: + Server: + - nginx + Date: + - Thu, 06 Jan 2022 14:53:09 GMT + Content-Type: + - text/html; charset=UTF-8 + Content-Length: + - '1583' + X-Request-Id: + - 27892c10-cc06-4b63-80ea-b14428d4585c + X-Runtime: + - '0.003955' + Strict-Transport-Security: + - max-age=63072000 + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +recorded_with: VCR 6.0.0 diff --git a/spec/fixtures/cassettes/archive/old_file_to_get_200.yml b/spec/fixtures/cassettes/archive/old_file_to_get_200.yml new file mode 100644 index 000000000..b35f5e1f5 --- /dev/null +++ b/spec/fixtures/cassettes/archive/old_file_to_get_200.yml @@ -0,0 +1,157 @@ +--- +http_interactions: +- request: + method: get + uri: https://opengraph.githubassets.com/5e61989aecb78e369c93674f877d7bf4ecde378850114a9563cdf8b6a2472536/typhoeus/typhoeus/issues/110 + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 200 + message: '' + headers: + X-Ratelimit-Limit: + - '100' + X-Ratelimit-Remaining: + - '83' + X-Ratelimit-Reset: + - '1641476165' + Access-Control-Allow-Origin: + - "*" + Content-Security-Policy: + - default-src 'none';style-src 'unsafe-inline';font-src https://github.github.com;img-src + https://avatars.githubusercontent.com https://github.githubassets.com https://camo.githubusercontent.com + X-Dns-Prefetch-Control: + - 'off' + Expect-Ct: + - max-age=0 + X-Frame-Options: + - SAMEORIGIN + X-Download-Options: + - noopen + X-Content-Type-Options: + - nosniff + X-Permitted-Cross-Domain-Policies: + - none + Referrer-Policy: + - no-referrer + X-Xss-Protection: + - '0' + Cache-Control: + - public, max-age=21600, immutable + Content-Type: + - image/png + Etag: + - W/"1066a-R45DSLyb/5W3DaEyAWdsUpLVmr4" + X-Github-Backend: + - Kubernetes + X-Github-Request-Id: + - CB64:6748:E2B78:86C62A:61D6EE15 + Via: + - 1.1 varnish, 1.1 varnish + Accept-Ranges: + - bytes + Date: + - Thu, 06 Jan 2022 14:38:47 GMT + Age: + - '4322' + X-Served-By: + - cache-iad-kiad7000110-IAD, cache-cdg20721-CDG + X-Cache: + - MISS, HIT + X-Cache-Hits: + - 0, 1 + Strict-Transport-Security: + - max-age=31536000 + X-Fastly-Request-Id: + - b6b7ae28f1e40734296daed0187e36df9f25de8d + Content-Length: + - '67178' + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +- request: + method: get + uri: https://opengraph.githubassets.com/5e61989aecb78e369c93674f877d7bf4ecde378850114a9563cdf8b6a2472536/typhoeus/typhoeus/issues/110 + body: + encoding: US-ASCII + string: '' + headers: + User-Agent: + - demarches-simplifiees.fr + Expect: + - '' + response: + status: + code: 200 + message: '' + headers: + X-Ratelimit-Limit: + - '100' + X-Ratelimit-Remaining: + - '83' + X-Ratelimit-Reset: + - '1641476165' + Access-Control-Allow-Origin: + - "*" + Content-Security-Policy: + - default-src 'none';style-src 'unsafe-inline';font-src https://github.github.com;img-src + https://avatars.githubusercontent.com https://github.githubassets.com https://camo.githubusercontent.com + X-Dns-Prefetch-Control: + - 'off' + Expect-Ct: + - max-age=0 + X-Frame-Options: + - SAMEORIGIN + X-Download-Options: + - noopen + X-Content-Type-Options: + - nosniff + X-Permitted-Cross-Domain-Policies: + - none + Referrer-Policy: + - no-referrer + X-Xss-Protection: + - '0' + Cache-Control: + - public, max-age=21600, immutable + Content-Type: + - image/png + Etag: + - W/"1066a-R45DSLyb/5W3DaEyAWdsUpLVmr4" + X-Github-Backend: + - Kubernetes + X-Github-Request-Id: + - CB64:6748:E2B78:86C62A:61D6EE15 + Via: + - 1.1 varnish, 1.1 varnish + Accept-Ranges: + - bytes + Date: + - Thu, 06 Jan 2022 14:38:47 GMT + Age: + - '4322' + X-Served-By: + - cache-iad-kiad7000110-IAD, cache-cdg20767-CDG + X-Cache: + - MISS, HIT + X-Cache-Hits: + - 0, 1 + Strict-Transport-Security: + - max-age=31536000 + X-Fastly-Request-Id: + - 58c062baca0e760a7a6c348ab9c64cceb965dd1a + Content-Length: + - '67178' + body: + encoding: ASCII-8BIT + string: '' + recorded_at: Wed, 04 Mar 2020 23:00:00 GMT +recorded_with: VCR 6.0.0 diff --git a/spec/lib/utils/retryable_spec.rb b/spec/lib/utils/retryable_spec.rb deleted file mode 100644 index 5e556713d..000000000 --- a/spec/lib/utils/retryable_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -describe Utils::Retryable do - Includer = Struct.new(:something) do - include Utils::Retryable - - def caller(max_attempt:, errors:) - with_retry(max_attempt: max_attempt, errors: errors) do - yield - end - end - end - - subject { Includer.new("test") } - let(:spy) { double() } - - describe '#with_retry' do - it 'works while retry count is less than max attempts' do - divider_that_raise_error = 0 - divider_that_works = 1 - expect(spy).to receive(:divider).and_return(divider_that_raise_error, divider_that_works) - result = subject.caller(max_attempt: 2, errors: [ZeroDivisionError]) { 10 / spy.divider } - expect(result).to eq(10 / divider_that_works) - end - - it 're raise error if it occures more than max_attempt' do - expect(spy).to receive(:divider).and_return(0, 0) - expect { subject.caller(max_attempt: 1, errors: [ZeroDivisionError]) { 0 / spy.divider } } - .to raise_error(ZeroDivisionError) - end - - it 'does not retry other errors' do - expect(spy).to receive(:divider).and_raise(StandardError).once - expect { subject.caller(max_attempt: 2, errors: [ZeroDivisionError]) { 0 / spy.divider } } - .to raise_error(StandardError) - end - end -end diff --git a/spec/services/procedure_archive_service_spec.rb b/spec/services/procedure_archive_service_spec.rb index 43f5efedf..97ab7b735 100644 --- a/spec/services/procedure_archive_service_spec.rb +++ b/spec/services/procedure_archive_service_spec.rb @@ -128,12 +128,11 @@ describe ProcedureArchiveService do let(:archive) { create(:archive, time_span_type: 'monthly', status: 'pending', month: date_month) } let(:year) { 2021 } let(:mailer) { double('mailer', deliver_later: true) } - before do - allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("http://file.to/get.ext") - end - it 'collect files' do + + it 'collects files with success' do + allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://opengraph.githubassets.com/d0e7862b24d8026a3c03516d865b28151eb3859029c6c6c2e86605891fbdcd7a/socketry/async-io") expect(InstructeurMailer).to receive(:send_archive).and_return(mailer) - VCR.use_cassette('archive/file_to_get') do + VCR.use_cassette('archive/new_file_to_get_200') do service.collect_files_archive(archive, instructeur) end @@ -152,6 +151,26 @@ describe ProcedureArchiveService do expect(archive.file.attached?).to be_truthy end + it 'retry errors files with errors' do + allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://www.demarches-simplifiees.fr/error_1") + expect(InstructeurMailer).to receive(:send_archive).and_return(mailer) + VCR.use_cassette('archive/new_file_to_get_400.html') do + service.collect_files_archive(archive, instructeur) + end + archive.file.open do |f| + files = ZipTricks::FileReader.read_zip_structure(io: f) + structure = [ + "procedure-#{procedure.id}/", + "procedure-#{procedure.id}/dossier-#{dossier.id}/", + "procedure-#{procedure.id}/dossier-#{dossier.id}/pieces_justificatives/", + "procedure-#{procedure.id}/dossier-#{dossier.id}/export-#{dossier.id}-05-03-2021-00-00-#{dossier.id}.pdf", + "procedure-#{procedure.id}/LISEZMOI.txt" + ] + expect(files.map(&:filename)).to match_array(structure) + end + expect(archive.file.attached?).to be_truthy + end + context 'with a missing file' do let(:pj) do PiecesJustificativesService::FakeAttachment.new( @@ -211,14 +230,12 @@ describe ProcedureArchiveService do context 'for all months' do let(:archive) { create(:archive, time_span_type: 'everything', status: 'pending') } let(:mailer) { double('mailer', deliver_later: true) } - before do - allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://i.etsystatic.com/6212702/r/il/744d2c/470726480/il_1588xN.470726480_bpk5.jpg") - end it 'collect files' do + allow_any_instance_of(ActiveStorage::Attached::One).to receive(:url).and_return("https://opengraph.githubassets.com/5e61989aecb78e369c93674f877d7bf4ecde378850114a9563cdf8b6a2472536/typhoeus/typhoeus/issues/110") expect(InstructeurMailer).to receive(:send_archive).and_return(mailer) - VCR.use_cassette('archive/file_to_get_typhoeus') do + VCR.use_cassette('archive/old_file_to_get_200') do service.collect_files_archive(archive, instructeur) end From fb75a55b89e07c1addfb7c2a1b644e646d4aa46e Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 5 Jan 2022 16:19:26 +0100 Subject: [PATCH 04/17] Add specs for Administrateurs::ProcedureAdministrateursController --- ...ocedure_administrateurs_controller_spec.rb | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb diff --git a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb new file mode 100644 index 000000000..bbec91374 --- /dev/null +++ b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb @@ -0,0 +1,26 @@ +describe Administrateurs::ProcedureAdministrateursController, type: :controller do + let(:signed_in_admin) { create(:administrateur) } + let(:other_admin) { create(:administrateur) } + let(:procedure) { create(:procedure, administrateurs: [signed_in_admin, other_admin]) } + + before do + sign_in(signed_in_admin.user) + end + + describe '#destroy' do + subject do + delete :destroy, params: { procedure_id: procedure.id, id: admin_to_remove.id }, format: :js, xhr: true + end + + context 'when removing another admin' do + let(:admin_to_remove) { other_admin } + + it 'removes the admin from the procedure' do + subject + expect(response.status).to eq(200) + expect(signed_in_admin.procedures.reload).to include(procedure) + expect(other_admin.procedures.reload).not_to include(procedure) + end + end + end +end From 062f52feb9be64debe6d6f2a732fe606cd97a65a Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Wed, 5 Jan 2022 15:21:59 +0000 Subject: [PATCH 05/17] Fix ProcedureAdministrateursController#destroy - The "no remove oneself" rule wasn't actually enforced - `flash.error` is not a flash category --- .../procedure_administrateurs_controller.rb | 3 ++- .../procedure_administrateurs_controller_spec.rb | 15 +++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/controllers/administrateurs/procedure_administrateurs_controller.rb b/app/controllers/administrateurs/procedure_administrateurs_controller.rb index c03c5c4a9..3bc56f546 100644 --- a/app/controllers/administrateurs/procedure_administrateurs_controller.rb +++ b/app/controllers/administrateurs/procedure_administrateurs_controller.rb @@ -32,7 +32,8 @@ module Administrateurs # Prevent self-removal (Also enforced in the UI) if administrateur == current_administrateur - flash.error = "Vous ne pouvez pas vous retirer vous-même d’une démarche." + flash.alert = "Vous ne pouvez pas vous retirer vous-même d’une démarche." + return end # Actually remove the admin diff --git a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb index bbec91374..2094a8d54 100644 --- a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb +++ b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb @@ -18,8 +18,19 @@ describe Administrateurs::ProcedureAdministrateursController, type: :controller it 'removes the admin from the procedure' do subject expect(response.status).to eq(200) - expect(signed_in_admin.procedures.reload).to include(procedure) - expect(other_admin.procedures.reload).not_to include(procedure) + expect(flash[:notice]).to be_present + expect(admin_to_remove.procedures.reload).not_to include(procedure) + end + end + + context 'when removing oneself from a procedure' do + let(:admin_to_remove) { signed_in_admin } + + it 'denies the right for an admin to remove itself' do + subject + expect(response.status).to eq(200) + expect(flash[:alert]).to be_present + expect(admin_to_remove.procedures.reload).to include(procedure) end end end From 967699c305a7e9f2241b414c2daeb1369cce7c49 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 11 Jan 2022 12:10:20 +0100 Subject: [PATCH 06/17] fix(select): improuve no results found message --- app/assets/stylesheets/forms.scss | 7 +++++++ app/javascript/components/ComboMultiple.jsx | 20 ++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/forms.scss b/app/assets/stylesheets/forms.scss index 031f96c37..596c1e2dd 100644 --- a/app/assets/stylesheets/forms.scss +++ b/app/assets/stylesheets/forms.scss @@ -535,6 +535,13 @@ padding: $default-spacer; } +[data-reach-combobox-no-results] { + font-size: 16px; + color: $dark-grey; + background: $light-grey; + padding: $default-spacer; +} + [data-reach-combobox-token] button { cursor: pointer; background-color: transparent; diff --git a/app/javascript/components/ComboMultiple.jsx b/app/javascript/components/ComboMultiple.jsx index 9b7d961ed..2faa4c156 100644 --- a/app/javascript/components/ComboMultiple.jsx +++ b/app/javascript/components/ComboMultiple.jsx @@ -196,13 +196,21 @@ function ComboMultiple({ {results && (results.length > 0 || !acceptNewValues) && ( - {results.length === 0 && ( -

- Aucun résultat{' '} - -

- )} + {results.length === 0 && ( +
  • + Aucun résultat{' '} + +
    • + )} {results.map(([label, value], index) => { if (label.startsWith('--')) { return ; From 4a718eae9229f8f1f075f199da0bd7d56984780a Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 11 Jan 2022 12:11:45 +0100 Subject: [PATCH 07/17] fix(select): always show menu on click in the input fix #6655 --- app/javascript/components/ComboMultiple.jsx | 23 ++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/app/javascript/components/ComboMultiple.jsx b/app/javascript/components/ComboMultiple.jsx index 2faa4c156..04ec02875 100644 --- a/app/javascript/components/ComboMultiple.jsx +++ b/app/javascript/components/ComboMultiple.jsx @@ -116,6 +116,7 @@ function ComboMultiple({ } setTerm(''); awaitFormSubmit.done(); + hidePopover(); }; const onRemove = (label) => { @@ -148,6 +149,18 @@ function ComboMultiple({ } }; + const hidePopover = () => { + document + .querySelector(`[data-reach-combobox-popover-id="${inputId}"]`) + ?.setAttribute('hidden', 'true'); + }; + + const showPopover = () => { + document + .querySelector(`[data-reach-combobox-popover-id="${inputId}"]`) + ?.removeAttribute('hidden'); + }; + const onBlur = () => { if ( term && @@ -156,6 +169,10 @@ function ComboMultiple({ awaitFormSubmit(() => { onSelect(term); }); + } else { + setTimeout(() => { + hidePopover(); + }, 200); } }; @@ -185,6 +202,7 @@ function ComboMultiple({ onChange={handleChange} onKeyDown={onKeyDown} onBlur={onBlur} + onClick={showPopover} autocomplete={false} id={inputId} aria-label={label} @@ -195,7 +213,10 @@ function ComboMultiple({ /> {results && (results.length > 0 || !acceptNewValues) && ( - + {results.length === 0 && (
  • From 006eac42403b09aab65c51c9c1dbcda39a7a9372 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:40:31 +0100 Subject: [PATCH 08/17] modify controller --- app/controllers/users/dossiers_controller.rb | 22 +++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/app/controllers/users/dossiers_controller.rb b/app/controllers/users/dossiers_controller.rb index ecad8100c..2b059e156 100644 --- a/app/controllers/users/dossiers_controller.rb +++ b/app/controllers/users/dossiers_controller.rb @@ -5,7 +5,7 @@ module Users layout 'procedure_context', only: [:identite, :update_identite, :siret, :update_siret] ACTIONS_ALLOWED_TO_ANY_USER = [:index, :recherche, :new, :transferer_all] - ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire] + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE = [:show, :demande, :messagerie, :brouillon, :update_brouillon, :modifier, :update, :create_commentaire, :restore] before_action :ensure_ownership!, except: ACTIONS_ALLOWED_TO_ANY_USER + ACTIONS_ALLOWED_TO_OWNER_OR_INVITE before_action :ensure_ownership_or_invitation!, only: ACTIONS_ALLOWED_TO_OWNER_OR_INVITE @@ -16,17 +16,18 @@ module Users before_action :store_user_location!, only: :new def index - @user_dossiers = current_user.dossiers.includes(:procedure).state_not_termine.order_by_updated_at.page(page) - @dossiers_traites = current_user.dossiers.includes(:procedure).state_termine.not_hidden_by_user.order_by_updated_at.page(page) + @user_dossiers = current_user.dossiers.includes(:procedure).state_not_termine.visible_by_user.order_by_updated_at.page(page) + @dossiers_traites = current_user.dossiers.includes(:procedure).state_termine.visible_by_user.order_by_updated_at.page(page) @dossiers_invites = current_user.dossiers_invites.includes(:procedure).order_by_updated_at.page(page) - @dossiers_supprimes = current_user.deleted_dossiers.order_by_updated_at.page(page) + @dossiers_supprimes_recemment = current_user.dossiers.hidden_by_user.order_by_updated_at.page(page) + @dossiers_supprimes_definitivement = current_user.deleted_dossiers.order_by_updated_at.page(page) @dossier_transfers = DossierTransfer .includes(dossiers: :user) .with_dossiers .where(email: current_user.email) .page(page) @dossiers_close_to_expiration = current_user.dossiers.close_to_expiration.page(page) - @statut = statut(@user_dossiers, @dossiers_traites, @dossiers_invites, @dossiers_supprimes, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) + @statut = statut(@user_dossiers, @dossiers_traites, @dossiers_invites, @dossiers_supprimes_recemment, @dossiers_supprimes_definitivement, @dossier_transfers, @dossiers_close_to_expiration, params[:statut]) end def show @@ -287,17 +288,24 @@ module Users @transfer = DossierTransfer.new(dossiers: current_user.dossiers) end + def restore + dossier.restore(current_user) + flash.notice = t('users.dossiers.restore') + redirect_to dossiers_path + end + private # if the status tab is filled, then this tab # else first filled tab # else en-cours - def statut(mes_dossiers, dossiers_traites, dossiers_invites, dossiers_supprimes, dossier_transfers, dossiers_close_to_expiration, params_statut) + def statut(mes_dossiers, dossiers_traites, dossiers_invites, dossiers_supprimes_recemment, dossiers_supprimes_definitivement, dossier_transfers, dossiers_close_to_expiration, params_statut) tabs = { 'en-cours' => mes_dossiers.present?, 'traites' => dossiers_traites.present?, 'dossiers-invites' => dossiers_invites.present?, - 'dossiers-supprimes' => dossiers_supprimes.present?, + 'dossiers-supprimes-recemment' => dossiers_supprimes_recemment.present?, + 'dossiers-supprimes-definitivement' => dossiers_supprimes_definitivement.present?, 'dossiers-transferes' => dossier_transfers.present?, 'dossiers-expirant' => dossiers_close_to_expiration.present? } From 894b065615d79ab9377b34b121bf00476b46894b Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:41:02 +0100 Subject: [PATCH 09/17] modify dossier model --- app/models/dossier.rb | 54 +++++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index f218cd412..d1417ccec 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -206,9 +206,10 @@ class Dossier < ApplicationRecord scope :archived, -> { where(archived: true) } scope :not_archived, -> { where(archived: false) } + scope :hidden_by_user, -> { where.not(hidden_by_user_at: nil) } scope :hidden_by_administration, -> { where.not(hidden_by_administration_at: nil) } - scope :not_hidden_by_user, -> { where(hidden_by_user_at: nil) } - scope :not_hidden_by_administration, -> { where(hidden_by_administration_at: nil) } + scope :visible_by_user, -> { where(hidden_by_user_at: nil) } + scope :visible_by_administration, -> { where("hidden_by_administration_at IS NULL AND NOT (hidden_by_user_at IS NOT NULL AND state = 'en_construction')") } scope :order_by_updated_at, -> (order = :desc) { order(updated_at: order) } scope :order_by_created_at, -> (order = :asc) { order(depose_at: order, created_at: order, id: order) } @@ -237,7 +238,7 @@ class Dossier < ApplicationRecord end scope :downloadable_sorted, -> { state_not_brouillon - .not_hidden_by_administration + .visible_by_administration .includes( :user, :individual, @@ -531,6 +532,10 @@ class Dossier < ApplicationRecord brouillon? || en_construction? || termine? end + def can_be_hidden_by_user? + en_construction? || termine? + end + def can_be_deleted_by_manager? kept? && can_be_deleted_by_user? end @@ -750,15 +755,25 @@ class Dossier < ApplicationRecord false end - def discard_and_keep_track!(author, reason) - author_is_user = author.is_a?(User) - author_is_administration = author.is_a?(Instructeur) || author.is_a?(Administrateur) || author.is_a?(SuperAdmin) + def author_is_user(author) + author.is_a?(User) + end - if termine? && author_is_administration + def author_is_administration(author) + author.is_a?(Instructeur) || author.is_a?(Administrateur) || author.is_a?(SuperAdmin) + end + + def restore_dossier_and_destroy_deleted_dossier(author) + deleted_dossier&.destroy! + log_dossier_operation(author, :restaurer, self) + end + + def discard_and_keep_track!(author, reason) + if termine? && author_is_administration(author) update(hidden_by_administration_at: Time.zone.now) end - if termine? && author_is_user + if can_be_hidden_by_user? && author_is_user(author) update(hidden_by_user_at: Time.zone.now) end @@ -766,14 +781,15 @@ class Dossier < ApplicationRecord deleted_dossier = nil transaction do - if deleted_by_instructeur_and_user? || en_construction? || brouillon? + if deleted_by_instructeur_and_user? || en_construction? && author_is_administration(author) || brouillon? if keep_track_on_deletion? log_dossier_operation(author, :supprimer, self) deleted_dossier = DeletedDossier.create_from_dossier(self, reason) end - update!(dossier_transfer_id: nil) - discard! + if !(en_construction? && author_is_user(author)) + discard! + end end end @@ -798,11 +814,19 @@ class Dossier < ApplicationRecord def restore(author) if discarded? transaction do - if hidden_by_administration? + if author_is_administration(author) && hidden_by_administration? update(hidden_by_administration_at: nil) - elsif undiscard && keep_track_on_deletion? - deleted_dossier&.destroy! - log_dossier_operation(author, :restaurer, self) + end + + if undiscard && keep_track_on_deletion? + restore_dossier_and_destroy_deleted_dossier(author) + end + end + elsif author_is_user(author) && hidden_by_user? + transaction do + update(hidden_by_user_at: nil) + if en_construction? + restore_dossier_and_destroy_deleted_dossier(author) end end end From 510e8f2fddc7b6c7ab6e2bc89ff35ca5d8711b15 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:41:18 +0100 Subject: [PATCH 10/17] routes --- config/routes.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/routes.rb b/config/routes.rb index ddde58659..9cd48ead5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -269,6 +269,7 @@ Rails.application.routes.draw do get 'messagerie' post 'commentaire' => 'dossiers#create_commentaire' patch 'delete_dossier' + patch 'restore', to: 'dossiers#restore' get 'attestation' get 'transferer', to: 'dossiers#transferer' end From c306387b00ba70835c6ab2831ee254ca6114629c Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:43:00 +0100 Subject: [PATCH 11/17] locales --- config/locales/en.yml | 13 +++++++++---- config/locales/fr.yml | 13 +++++++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index e8d25670f..ec96dd382 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -364,10 +364,14 @@ en: zero: guest file one: guest file other: guest files - dossiers_supprimes: - zero: deleted file - one: deleted file - other: deleted files + dossiers_supprimes_recemment: + zero: recently deleted file + one: recently deleted file + other: recently deleted files + dossiers_supprimes_definitivement: + zero: permanently deleted file + one: permanently deleted file + other: permanently deleted files dossiers_transferes: zero: transfer request one: transfer request @@ -402,6 +406,7 @@ en: ask_deletion: undergoingreview: "Your file is undergoing review. It is no longer possible to delete your file. To cancel the undergoingreview contact the adminitration via the mailbox." soft_deleted_dossier: "Your file has been successfully deleted from your interface" + restore: "Your file has been successfully restored" update_brouillon: draft_saved: "Your draft has been saved." etablissement: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index e5947ea20..49bed38a3 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -371,10 +371,14 @@ fr: zero: dossier invité one: dossier invité other: dossiers invités - dossiers_supprimes: - zero: dossier supprimé - one: dossier supprimé - other: dossiers supprimés + dossiers_supprimes_recemment: + zero: dossier supprimé recemment + one: dossier supprimé recemment + other: dossiers supprimés recemment + dossiers_supprimes_definitivement: + zero: dossier supprimé définitivement + one: dossier supprimé définitivement + other: dossiers supprimés définitivement dossiers_transferes: zero: demande de transfert one: demande de transfert @@ -410,6 +414,7 @@ fr: ask_deletion: undergoingreview: "L’instruction de votre dossier a commencé, il n’est plus possible de supprimer votre dossier. Si vous souhaitez annuler l’instruction contactez votre administration par la messagerie de votre dossier." soft_deleted_dossier: "Votre dossier a bien été supprimé de votre interface" + restore: "Votre dossier a bien été restauré" update_brouillon: draft_saved: "Votre brouillon a bien été sauvegardé." etablissement: From 46066502488b4a0344ec6f9e683a5a345988b8f5 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:43:13 +0100 Subject: [PATCH 12/17] layout --- .../dossiers/_deleted_dossiers_list.html.haml | 1 + .../dossiers/_hidden_dossiers_list.html.haml | 36 +++++++++++++++++++ app/views/users/dossiers/index.html.haml | 24 +++++++++---- 3 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 app/views/users/dossiers/_hidden_dossiers_list.html.haml diff --git a/app/views/users/dossiers/_deleted_dossiers_list.html.haml b/app/views/users/dossiers/_deleted_dossiers_list.html.haml index 4b1e039d2..55785b475 100644 --- a/app/views/users/dossiers/_deleted_dossiers_list.html.haml +++ b/app/views/users/dossiers/_deleted_dossiers_list.html.haml @@ -9,6 +9,7 @@ %tbody - deleted_dossiers.each do |dossier| - libelle_demarche = Procedure.find(dossier.procedure_id).libelle + %tr{ data: { 'dossier-id': dossier.dossier_id } } %td.number-col %span.icon.folder diff --git a/app/views/users/dossiers/_hidden_dossiers_list.html.haml b/app/views/users/dossiers/_hidden_dossiers_list.html.haml new file mode 100644 index 000000000..58f2f7eb1 --- /dev/null +++ b/app/views/users/dossiers/_hidden_dossiers_list.html.haml @@ -0,0 +1,36 @@ +- if hidden_dossiers.present? + %table.table.dossiers-table.hoverable + %thead + %tr + %th.number-col Nº dossier + %th Démarche + %th Raison de suppression + %th Date de suppression + %tbody + - hidden_dossiers.each do |dossier| + - libelle_demarche = dossier.procedure.libelle + + %tr{ data: { 'dossier-id': dossier.id } } + %td.number-col + %span.icon.folder + = dossier.id + %td + = libelle_demarche + + %td.cell-link + = deletion_reason_badge("user_request") + %td + = dossier.updated_at.strftime('%d/%m/%Y') + %td + = link_to restore_dossier_path(dossier.id), method: :patch, class: "button primary" do + Restaurer + + = paginate(hidden_dossiers) + +- else + .blank-tab + %h2.empty-text Aucun dossier. + %p.empty-text-details + Pour remplir une démarche, contactez votre administration en lui demandant le lien de la démarche. + %br + Celui ci doit ressembler à #{APPLICATION_BASE_URL}/commencer/xxx. diff --git a/app/views/users/dossiers/index.html.haml b/app/views/users/dossiers/index.html.haml index 326962e92..577625690 100644 --- a/app/views/users/dossiers/index.html.haml +++ b/app/views/users/dossiers/index.html.haml @@ -39,11 +39,17 @@ active: @statut == 'dossiers-expirant', badge: number_with_html_delimiter(@dossiers_close_to_expiration.count)) - - if @dossiers_supprimes.present? - = tab_item(t('pluralize.dossiers_supprimes', count: @dossiers_supprimes.count), - dossiers_path(statut: 'dossiers-supprimes'), - active: @statut == 'dossiers-supprimes', - badge: number_with_html_delimiter(@dossiers_supprimes.count)) + - if @dossiers_supprimes_recemment.present? + = tab_item(t('pluralize.dossiers_supprimes_recemment', count: @dossiers_supprimes_recemment.count), + dossiers_path(statut: 'dossiers-supprimes-recemment'), + active: @statut == 'dossiers-supprimes-recemment', + badge: number_with_html_delimiter(@dossiers_supprimes_recemment.count)) + + - if @dossiers_supprimes_definitivement.present? + = tab_item(t('pluralize.dossiers_supprimes_definitivement', count: @dossiers_supprimes_definitivement.count), + dossiers_path(statut: 'dossiers-supprimes-definitivement'), + active: @statut == 'dossiers-supprimes-definitivement', + badge: number_with_html_delimiter(@dossiers_supprimes_definitivement.count)) - if @dossier_transfers.present? = tab_item(t('pluralize.dossiers_transferes', count: @dossier_transfers.count), @@ -61,8 +67,12 @@ - if @statut == "dossiers-invites" = render partial: "dossiers_list", locals: { dossiers: @dossiers_invites } - - if @statut == "dossiers-supprimes" - = render partial: "deleted_dossiers_list", locals: { deleted_dossiers: @dossiers_supprimes } + - if @statut == "dossiers-supprimes-recemment" + = render partial: "hidden_dossiers_list", locals: { hidden_dossiers: @dossiers_supprimes_recemment } + + - if @statut == "dossiers-supprimes-definitivement" + = render partial: "deleted_dossiers_list", locals: { deleted_dossiers: @dossiers_supprimes_definitivement } + - if @statut == "dossiers-transferes" = render partial: "transfered_dossiers_list", locals: { dossier_transfers: @dossier_transfers } From bdcb0ca0e9bcd93d9a3743c675d82e8057e7f277 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Wed, 5 Jan 2022 10:43:20 +0100 Subject: [PATCH 13/17] tests --- .../users/dossiers_controller_spec.rb | 21 ++++++++++++++++--- spec/models/dossier_spec.rb | 18 +++++++++------- .../users/dossiers/index.html.haml_spec.rb | 5 +++-- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/spec/controllers/users/dossiers_controller_spec.rb b/spec/controllers/users/dossiers_controller_spec.rb index a66752026..5d278d665 100644 --- a/spec/controllers/users/dossiers_controller_spec.rb +++ b/spec/controllers/users/dossiers_controller_spec.rb @@ -1028,13 +1028,13 @@ describe Users::DossiersController, type: :controller do subject end - it "deletes the dossier" do + it "hide the dossier and create a deleted dossier" do procedure = dossier.procedure dossier_id = dossier.id subject - expect(Dossier.find_by(id: dossier_id)).to eq(nil) + expect(Dossier.find_by(id: dossier_id)).to be_present + expect(Dossier.find_by(id: dossier_id).hidden_by_user_at).to be_present expect(procedure.deleted_dossiers.count).to eq(1) - expect(procedure.deleted_dossiers.first.dossier_id).to eq(dossier_id) end it { is_expected.to redirect_to(dossiers_path) } @@ -1065,6 +1065,21 @@ describe Users::DossiersController, type: :controller do end end + describe '#restore' do + before { sign_in(user) } + subject { patch :restore, params: { id: dossier.id } } + + context 'when the user want to restore his dossier' do + let!(:dossier) { create(:dossier, :with_individual, state: :accepte, en_construction_at: Time.zone.yesterday.beginning_of_day.utc, hidden_by_user_at: Time.zone.yesterday.beginning_of_day.utc, user: user, autorisation_donnees: true) } + + before { subject } + + it 'must have hidden_by_user_at nil' do + expect(dossier.reload.hidden_by_user_at).to be_nil + end + end + end + describe '#new' do let(:procedure) { create(:procedure, :published) } let(:procedure_id) { procedure.id } diff --git a/spec/models/dossier_spec.rb b/spec/models/dossier_spec.rb index 96b924fc8..42a5ef2a9 100644 --- a/spec/models/dossier_spec.rb +++ b/spec/models/dossier_spec.rb @@ -793,6 +793,7 @@ describe Dossier do describe "#discard_and_keep_track!" do let(:dossier) { create(:dossier, :en_construction) } + let(:user) { dossier.user } let(:deleted_dossier) { DeletedDossier.find_by(dossier_id: dossier.id) } let(:last_operation) { dossier.dossier_operation_logs.last } let(:reason) { :user_request } @@ -802,7 +803,7 @@ describe Dossier do allow(DossierMailer).to receive(:notify_deletion_to_administration).and_return(double(deliver_later: nil)) end - subject! { dossier.discard_and_keep_track!(dossier.user, reason) } + subject! { dossier.discard_and_keep_track!(user, reason) } context 'brouillon' do let(:dossier) { create(:dossier) } @@ -821,8 +822,9 @@ describe Dossier do end context 'en_construction' do - it 'hides the dossier' do - expect(dossier.hidden_at).to be_present + it 'hide the dossier but does not discard' do + expect(dossier.hidden_at).to be_nil + expect(dossier.hidden_by_user_at).to be_present end it 'creates a DeletedDossier record' do @@ -872,6 +874,7 @@ describe Dossier do end context 'with reason: manager_request' do + let(:user) { dossier.procedure.administrateurs.first } let(:reason) { :manager_request } it 'hides the dossier' do @@ -887,13 +890,12 @@ describe Dossier do context 'with reason: user_removed' do let(:reason) { :user_removed } - it 'hides the dossier' do - expect(dossier.discarded?).to be_truthy + it 'does not discard the dossier' do + expect(dossier.discarded?).to be_falsy end - it 'records the operation in the log' do - expect(last_operation.operation).to eq("supprimer") - expect(last_operation.automatic_operation?).to be_falsey + it 'hide the dossier' do + expect(dossier.hidden_by_user_at).to be_present end end end diff --git a/spec/views/users/dossiers/index.html.haml_spec.rb b/spec/views/users/dossiers/index.html.haml_spec.rb index 5d74ff091..79ee66595 100644 --- a/spec/views/users/dossiers/index.html.haml_spec.rb +++ b/spec/views/users/dossiers/index.html.haml_spec.rb @@ -12,7 +12,8 @@ describe 'users/dossiers/index.html.haml', type: :view do allow(controller).to receive(:current_user) { user } assign(:user_dossiers, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossiers_invites, Kaminari.paginate_array(dossiers_invites).page(1)) - assign(:dossiers_supprimes, Kaminari.paginate_array(user_dossiers).page(1)) + assign(:dossiers_supprimes_recemment, Kaminari.paginate_array(user_dossiers).page(1)) + assign(:dossiers_supprimes_definitivement, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossiers_traites, Kaminari.paginate_array(user_dossiers).page(1)) assign(:dossier_transfers, Kaminari.paginate_array([]).page(1)) assign(:dossiers_close_to_expiration, Kaminari.paginate_array([]).page(1)) @@ -69,7 +70,7 @@ describe 'users/dossiers/index.html.haml', type: :view do it 'affiche la barre d’onglets' do expect(rendered).to have_selector('nav.tabs') - expect(rendered).to have_selector('nav.tabs li', count: 4) + expect(rendered).to have_selector('nav.tabs li', count: 5) expect(rendered).to have_selector('nav.tabs li.active', count: 1) end end From e2caf5718fa5d941b2a6c7062550a0d792571355 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 10 Jan 2022 15:11:00 +0100 Subject: [PATCH 14/17] modify instructeur model --- app/models/instructeur.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/instructeur.rb b/app/models/instructeur.rb index c5ea07183..44e199b33 100644 --- a/app/models/instructeur.rb +++ b/app/models/instructeur.rb @@ -232,7 +232,7 @@ class Instructeur < ApplicationRecord def dossiers_count_summary(groupe_instructeur_ids) query = <<~EOF SELECT - COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.id IS NULL) AS a_suivre, + COUNT(DISTINCT dossiers.id) FILTER (where not archived AND NOT (dossiers.hidden_by_user_at IS NOT NULL AND state = 'en_construction') AND dossiers.state in ('en_construction', 'en_instruction') AND follows.id IS NULL) AS a_suivre, COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('en_construction', 'en_instruction') AND follows.instructeur_id = :instructeur_id) AS suivis, COUNT(DISTINCT dossiers.id) FILTER (where not archived AND dossiers.state in ('accepte', 'refuse', 'sans_suite')) AS traites, COUNT(DISTINCT dossiers.id) FILTER (where not archived) AS tous, From 68b45ae31d236d559ba439850bfe173d3f602437 Mon Sep 17 00:00:00 2001 From: Kara Diaby Date: Mon, 10 Jan 2022 15:11:34 +0100 Subject: [PATCH 15/17] modify instructeur procedures controller --- app/controllers/instructeurs/procedures_controller.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/instructeurs/procedures_controller.rb b/app/controllers/instructeurs/procedures_controller.rb index b221cf43a..210dcc3fe 100644 --- a/app/controllers/instructeurs/procedures_controller.rb +++ b/app/controllers/instructeurs/procedures_controller.rb @@ -14,9 +14,9 @@ module Instructeurs dossiers = current_instructeur.dossiers.joins(:groupe_instructeur) @dossiers_count_per_procedure = dossiers.all_state.group('groupe_instructeurs.procedure_id').reorder(nil).count - @dossiers_a_suivre_count_per_procedure = dossiers.without_followers.en_cours.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_a_suivre_count_per_procedure = dossiers.without_followers.en_cours.visible_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count @dossiers_archived_count_per_procedure = dossiers.archived.group('groupe_instructeurs.procedure_id').count - @dossiers_termines_count_per_procedure = dossiers.termine.not_hidden_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count + @dossiers_termines_count_per_procedure = dossiers.termine.visible_by_administration.group('groupe_instructeurs.procedure_id').reorder(nil).count @dossiers_expirant_count_per_procedure = dossiers.termine_or_en_construction_close_to_expiration.group('groupe_instructeurs.procedure_id').count groupe_ids = current_instructeur.groupe_instructeurs.pluck(:id) @@ -61,6 +61,7 @@ module Instructeurs @a_suivre_dossiers = dossiers_visibles .without_followers .en_cours + .visible_by_administration @followed_dossiers = current_instructeur .followed_dossiers @@ -69,7 +70,7 @@ module Instructeurs @followed_dossiers_id = @followed_dossiers.pluck(:id) - @termines_dossiers = dossiers_visibles.termine.not_hidden_by_administration + @termines_dossiers = dossiers_visibles.termine.visible_by_administration @all_state_dossiers = dossiers_visibles.all_state @archived_dossiers = dossiers_visibles.archived @expirant_dossiers = dossiers_visibles.termine_or_en_construction_close_to_expiration From 7e0d471a080ee54f9926696452642b85f5d331a7 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 11 Jan 2022 15:20:23 +0100 Subject: [PATCH 16/17] improuve discarded_expired scopes --- app/models/dossier.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/app/models/dossier.rb b/app/models/dossier.rb index d1417ccec..453ad302a 100644 --- a/app/models/dossier.rb +++ b/app/models/dossier.rb @@ -347,23 +347,26 @@ class Dossier < ApplicationRecord scope :without_en_construction_expiration_notice_sent, -> { where(en_construction_close_to_expiration_notice_sent_at: nil) } scope :without_termine_expiration_notice_sent, -> { where(termine_close_to_expiration_notice_sent_at: nil) } + scope :discarded_expired, -> { discarded.where('dossiers.hidden_at < ?', 1.week.ago) } + scope :discarded_by_user_expired, -> { discarded.where('dossiers.hidden_by_user_at < ?', 1.week.ago) } + scope :discarded_by_administration_expired, -> { discarded.where('dossiers.hidden_by_administration_at < ?', 1.week.ago) } scope :discarded_brouillon_expired, -> do with_discarded - .discarded .state_brouillon - .where('hidden_at < ?', 1.week.ago) + .discarded_expired + .or(state_brouillon.discarded_by_user_expired) end scope :discarded_en_construction_expired, -> do with_discarded - .discarded .state_en_construction - .where('dossiers.hidden_at < ?', 1.week.ago) + .discarded_expired + .or(state_en_construction.discarded_by_user_expired) end scope :discarded_termine_expired, -> do with_discarded - .discarded .state_termine - .where('dossiers.hidden_at < ?', 1.week.ago) + .discarded_expired + .or(state_termine.discarded_by_user_expired.discarded_by_administration_expired) end scope :brouillon_near_procedure_closing_date, -> do @@ -774,14 +777,14 @@ class Dossier < ApplicationRecord end if can_be_hidden_by_user? && author_is_user(author) - update(hidden_by_user_at: Time.zone.now) + update(hidden_by_user_at: Time.zone.now, dossier_transfer_id: nil) end user_email = user_deleted? ? nil : user_email_for(:notification) deleted_dossier = nil transaction do - if deleted_by_instructeur_and_user? || en_construction? && author_is_administration(author) || brouillon? + if deleted_by_instructeur_and_user? || en_construction? || brouillon? if keep_track_on_deletion? log_dossier_operation(author, :supprimer, self) deleted_dossier = DeletedDossier.create_from_dossier(self, reason) From baa867124b79fa382c35b602872819756601a8f6 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 13 Jan 2022 11:06:28 +0000 Subject: [PATCH 17/17] build(deps): bump follow-redirects from 1.14.6 to 1.14.7 Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.6 to 1.14.7. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](https://github.com/follow-redirects/follow-redirects/compare/v1.14.6...v1.14.7) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index bf48941f5..87b168209 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6038,9 +6038,9 @@ folder-walker@^3.2.0: from2 "^2.1.0" follow-redirects@^1.0.0: - version "1.14.6" - resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.14.6.tgz#8cfb281bbc035b3c067d6cd975b0f6ade6e855cd" - integrity sha512-fhUl5EwSJbbl8AR+uYL2KQDxLkdSjZGR36xy46AO7cOMTrCMON6Sa28FmAnC2tRTDbd/Uuzz3aJBv7EBN7JH8A== + version "1.14.7" + resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.14.7.tgz#2004c02eb9436eee9a21446a6477debf17e81685" + integrity sha512-+hbxoLbFMbRKDwohX8GkTataGqO6Jb7jGwpAlwgy2bIz25XtRm7KEzJM76R1WiNT5SwZkX4Y75SwBolkpmE7iQ== for-in@^1.0.2: version "1.0.2"