From 14a2be00e2635bc86a58100d844dc47c4bbcd8c2 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 24 Feb 2020 10:23:16 +0100 Subject: [PATCH 01/14] 4806 - ajout de precisions sur l'export CSV --- .../instructeurs/procedures/_download_dossiers.html.haml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/views/instructeurs/procedures/_download_dossiers.html.haml b/app/views/instructeurs/procedures/_download_dossiers.html.haml index f07fd6c03..a466d3cef 100644 --- a/app/views/instructeurs/procedures/_download_dossiers.html.haml +++ b/app/views/instructeurs/procedures/_download_dossiers.html.haml @@ -4,12 +4,14 @@ Télécharger tous les dossiers .dropdown-content.fade-in-down{ style: 'width: 330px' } %ul.dropdown-items - - [[xlsx_export, :xlsx], [csv_export, :csv], [ods_export, :ods]].each do |(export, format)| + - [[xlsx_export, :xlsx], [ods_export, :ods], [csv_export, :csv]].each do |(export, format)| %li - if export.nil? - = link_to "Demander un export au format .#{format}", download_export_instructeur_procedure_path(procedure, export_format: format), remote: true + - export_text = "Demander un export au format .#{format}" + - if format == :csv + - export_text = "Demander un export au format .#{format}
(uniquement les dossiers, sans les champs répétables)".html_safe + = link_to export_text, download_export_instructeur_procedure_path(procedure, export_format: format), remote: true - elsif export.ready? = link_to "Télécharger l'export au format .#{format}", export.file.service_url, target: "_blank", rel: "noopener" - else %span{ 'data-export-poll-url': download_export_instructeur_procedure_path(procedure, export_format: format, no_progress_notification: true) } - L'export au format .#{format} est en cours de préparation From ae14599245a452c6576a3c89b4a736dd111c7b7c Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Mon, 24 Feb 2020 10:33:31 +0100 Subject: [PATCH 02/14] #4700: export files are antivirus-safe --- app/models/export.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/export.rb b/app/models/export.rb index 820b15ee8..a67b835b7 100644 --- a/app/models/export.rb +++ b/app/models/export.rb @@ -25,7 +25,9 @@ class Export < ApplicationRecord file.attach( io: io, filename: filename, - content_type: content_type + content_type: content_type, + # We generate the exports ourselves, so they are safe + metadata: { virus_scan_result: ActiveStorage::VirusScanner::SAFE } ) end From 480765aa17b456162cef77e7131a89ed3581b932 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 24 Feb 2020 11:32:52 +0100 Subject: [PATCH 03/14] procedure: use smaller headers in the edit form --- .../stylesheets/new_design/procedure_form.scss | 2 +- .../procedures/_informations.html.haml | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/new_design/procedure_form.scss b/app/assets/stylesheets/new_design/procedure_form.scss index 4c920eb59..3b71a0f0d 100644 --- a/app/assets/stylesheets/new_design/procedure_form.scss +++ b/app/assets/stylesheets/new_design/procedure_form.scss @@ -73,7 +73,7 @@ .procedure-form__options-summary { cursor: pointer; - .header-section { + .header-subsection { display: inline-block; } } diff --git a/app/views/new_administrateur/procedures/_informations.html.haml b/app/views/new_administrateur/procedures/_informations.html.haml index be435258a..0adf4c98c 100644 --- a/app/views/new_administrateur/procedures/_informations.html.haml +++ b/app/views/new_administrateur/procedures/_informations.html.haml @@ -13,7 +13,7 @@ %span.mandatory * = f.text_area :description, rows: '6', placeholder: 'Description de la démarche, destinataires, etc. ', class: 'form-control' -%h2.header-section Logo de la démarche +%h3.header-subsection Logo de la démarche = render 'shared/attachment/edit', { form: f, attached_file: @procedure.logo, @@ -21,7 +21,7 @@ user_can_destroy: true } - if !@procedure.locked? - %h2.header-section Conservation des données + %h3.header-subsection Conservation des données = f.label :duree_conservation_dossiers_dans_ds do Sur demarches-simplifiees.fr %span.mandatory * @@ -39,7 +39,7 @@ Où les usagers trouveront-ils le lien vers la démarche ? = f.text_field :lien_site_web, class: 'form-control', placeholder: 'https://exemple.gouv.fr/ma_demarche' -%h2.header-section +%h3.header-subsection Cadre juridique %span.mandatory * @@ -64,7 +64,7 @@ attached_file: @procedure.deliberation, user_can_destroy: true } -%h2.header-section Notice explicative de la démarche +%h3.header-subsection Notice explicative de la démarche %p.notice Une notice explicative est un document destiné à guider l’usager dans sa démarche. C’est un document que vous avez élaboré et qui peut prendre la forme d’un fichier doc, d’un pdf ou encore de diapositives. Le bouton pour télécharger cette notice apparaît en haut du formulaire pour l’usager. @@ -77,7 +77,7 @@ user_can_destroy: true } - if !@procedure.locked? - %h2.header-section À qui s’adresse ma démarche ? + %h3.header-subsection À qui s’adresse ma démarche ? .radios.vertical = f.label :for_individual, value: true do = f.radio_button :for_individual, true @@ -95,7 +95,7 @@ %details.procedure-form__options-details %summary.procedure-form__options-summary - %h2.header-section Options avancées + %h3.header-subsection Options avancées - if feature_enabled?(:administrateur_web_hook) = f.label :web_hook_url do From cdf48bcfa6d09194d32a001ff817382d770acf1f Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 24 Feb 2020 11:35:25 +0100 Subject: [PATCH 04/14] procedure: add a default label to the declarative prompt --- app/views/new_administrateur/procedures/_informations.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/new_administrateur/procedures/_informations.html.haml b/app/views/new_administrateur/procedures/_informations.html.haml index 0adf4c98c..83bca48fd 100644 --- a/app/views/new_administrateur/procedures/_informations.html.haml +++ b/app/views/new_administrateur/procedures/_informations.html.haml @@ -115,7 +115,7 @@ = f.label :declarative_with_state do Démarche déclarative - = f.select :declarative_with_state, Procedure.declarative_attributes_for_select, { include_blank: true }, class: 'form-control' + = f.select :declarative_with_state, Procedure.declarative_attributes_for_select, { prompt: 'Non' }, class: 'form-control' %p.explication Par défaut, une démarche n'est pas déclarative; à son dépot, un dossier est «en construction». Vous pouvez choisir de la rendre déclarative, afin que tous les dossiers déposés soient immédiatement au statut "en instruction" en "accepté". From d49217bd72db6a2a92aad8d4e079c25c736ba094 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 24 Feb 2020 11:38:22 +0100 Subject: [PATCH 05/14] procedure: improve wording for declarative procedures --- .../new_administrateur/procedures/_informations.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/new_administrateur/procedures/_informations.html.haml b/app/views/new_administrateur/procedures/_informations.html.haml index 83bca48fd..ee3e8850d 100644 --- a/app/views/new_administrateur/procedures/_informations.html.haml +++ b/app/views/new_administrateur/procedures/_informations.html.haml @@ -118,7 +118,7 @@ = f.select :declarative_with_state, Procedure.declarative_attributes_for_select, { prompt: 'Non' }, class: 'form-control' %p.explication - Par défaut, une démarche n'est pas déclarative; à son dépot, un dossier est «en construction». Vous pouvez choisir de la rendre déclarative, afin que tous les dossiers déposés soient immédiatement au statut "en instruction" en "accepté". + Par défaut, une démarche n’est pas déclarative ; à son dépôt, un dossier est « en construction ». Vous pouvez choisir de la rendre déclarative, afin que tous les dossiers déposés passent immédiatement au statut « en instruction » ou « accepté ». %br %br - Dans le cadre d'une démarche déclarative, au dépot, seul l'email associé à l'état choisi est envoyé. (ex: démarche déclarative «accepté»: envoi uniquement de l'email d'acceptation) + Dans le cadre d’une démarche déclarative, au dépôt, seul l’email associé à l’état choisi est envoyé. (ex: démarche déclarative « accepté » : envoi uniquement de l’email d'acceptation) From 0b06864f7afcf571f0fee82a517fec76f17f0afc Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 30 Jan 2020 13:40:06 +0100 Subject: [PATCH 06/14] Upgrade core-js --- app/javascript/shared/polyfills.js | 7 ++++--- babel.config.js | 4 ++-- config/environments/development.rb | 3 +++ package.json | 2 +- yarn.lock | 10 +++++----- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/javascript/shared/polyfills.js b/app/javascript/shared/polyfills.js index 24b086ccc..d08eb91bf 100644 --- a/app/javascript/shared/polyfills.js +++ b/app/javascript/shared/polyfills.js @@ -1,9 +1,10 @@ // Include runtime-polyfills for older browsers. // Due to babel.config.js's 'useBuiltIns', only polyfills actually // required by the browsers we support will be included. -import '@babel/polyfill'; +import 'core-js/stable'; +import 'regenerator-runtime/runtime'; import 'dom4'; +import 'intersection-observer'; + import './polyfills/insertAdjacentElement'; import './polyfills/dataset'; - -import('intersection-observer'); diff --git a/babel.config.js b/babel.config.js index 61ca37797..9c0040b9a 100644 --- a/babel.config.js +++ b/babel.config.js @@ -29,8 +29,8 @@ module.exports = function(api) { require('@babel/preset-env').default, { forceAllTransforms: true, - useBuiltIns: 'entry', - corejs: 2, + useBuiltIns: 'usage', + corejs: 3, modules: false, exclude: ['transform-typeof-symbol'] } diff --git a/config/environments/development.rb b/config/environments/development.rb index 718db3436..82ee10641 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -39,6 +39,9 @@ Rails.application.configure do # yet still be able to expire them through the digest params. config.assets.digest = true + # Suppress logger output for asset requests. + config.assets.quiet = true + # Adds additional error checking when serving assets at runtime. # Checks for improperly declared sprockets dependencies. # Raises helpful error messages. diff --git a/package.json b/package.json index 70a46b48c..01656a298 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "babel-plugin-macros": "^2.8.0", "babel-plugin-transform-react-remove-prop-types": "^0.4.24", "chartkick": "^3.2.0", - "core-js": "^2.0.0", + "core-js": "^3.6.4", "debounce": "^1.2.0", "dom4": "^2.1.5", "email-butler": "^1.0.13", diff --git a/yarn.lock b/yarn.lock index b17ed8b91..501bc0a24 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2572,16 +2572,16 @@ core-js-compat@^3.6.0: browserslist "^4.8.3" semver "7.0.0" -core-js@^2.0.0: - version "2.6.9" - resolved "https://registry.yarnpkg.com/core-js/-/core-js-2.6.9.tgz#6b4b214620c834152e179323727fc19741b084f2" - integrity sha512-HOpZf6eXmnl7la+cUdMnLvUxKNqLUzJvgIziQ0DiF3JwSImNphIqdGqzj6hIKyX04MmV0poclQ7+wjWvxQyR2A== - core-js@^3.4.0: version "3.6.2" resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.6.2.tgz#2799ea1a59050f0acf50dfe89b916d6503b16caa" integrity sha512-hIE5dXkRzRvnZ5vhkRfQxUvDxQZmD9oueA08jDYRBKJHx+VIl/Pne/e0A4x9LObEEthC/TqiZybUoNM4tRgnKg== +core-js@^3.6.4: + version "3.6.4" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.6.4.tgz#440a83536b458114b9cb2ac1580ba377dc470647" + integrity sha512-4paDGScNgZP2IXXilaffL9X7968RuvwlkK3xWtZRVqgd8SYNiVKRJvkFd1aqqEuPfN7E68ZHEp9hDj6lHj4Hyw== + core-util-is@1.0.2, core-util-is@~1.0.0: version "1.0.2" resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.2.tgz#b5fd54220aa2bc5ab57aab7140c940754503c1a7" From d8eb3ebb9411ccaf172d1c75ae8ec33829f93e71 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 Feb 2020 19:01:23 +0100 Subject: [PATCH 07/14] Load chartkick and highcharts only on stat pages --- Gemfile.lock | 2 +- .../toggle-chart.js => components/Chartkick.js} | 15 +++++++++++++-- app/javascript/loaders/Chartkick.js | 3 +++ app/javascript/packs/application.js | 6 ------ app/views/instructeurs/procedures/stats.html.haml | 2 ++ app/views/stats/index.html.haml | 9 +++++---- package.json | 2 +- yarn.lock | 8 ++++---- 8 files changed, 29 insertions(+), 18 deletions(-) rename app/javascript/{new_design/toggle-chart.js => components/Chartkick.js} (72%) create mode 100644 app/javascript/loaders/Chartkick.js diff --git a/Gemfile.lock b/Gemfile.lock index 13f80a794..33c623920 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,7 +138,7 @@ GEM selenium-webdriver case_transform (0.2) activesupport - chartkick (3.3.0) + chartkick (3.3.1) childprocess (0.9.0) ffi (~> 1.0, >= 1.0.11) chunky_png (1.3.11) diff --git a/app/javascript/new_design/toggle-chart.js b/app/javascript/components/Chartkick.js similarity index 72% rename from app/javascript/new_design/toggle-chart.js rename to app/javascript/components/Chartkick.js index 2de8b6394..e9d5e2e6a 100644 --- a/app/javascript/new_design/toggle-chart.js +++ b/app/javascript/components/Chartkick.js @@ -1,8 +1,14 @@ import Chartkick from 'chartkick'; -import { toggle } from '@utils'; +import Highcharts from 'highcharts'; +import { toggle, delegate } from '@utils'; -export function toggleChart(event, chartClass) { +export default function() { + return null; +} + +function toggleChart(event) { const nextSelectorItem = event.target, + chartClass = event.target.dataset.toggleChart, nextChart = document.querySelector(chartClass), nextChartId = nextChart.children[0].id, currentSelectorItem = nextSelectorItem.parentElement.querySelector( @@ -23,3 +29,8 @@ export function toggleChart(event, chartClass) { // Reflow needed, see https://github.com/highcharts/highcharts/issues/1979 Chartkick.charts[nextChartId].getChartObject().reflow(); } + +delegate('click', '[data-toggle-chart]', toggleChart); + +Chartkick.use(Highcharts); +window.Chartkick = Chartkick; diff --git a/app/javascript/loaders/Chartkick.js b/app/javascript/loaders/Chartkick.js new file mode 100644 index 000000000..0be342493 --- /dev/null +++ b/app/javascript/loaders/Chartkick.js @@ -0,0 +1,3 @@ +import Loadable from '../components/Loadable'; + +export default Loadable(() => import('../components/Chartkick')); diff --git a/app/javascript/packs/application.js b/app/javascript/packs/application.js index d19723244..7689c88d6 100644 --- a/app/javascript/packs/application.js +++ b/app/javascript/packs/application.js @@ -4,8 +4,6 @@ import Rails from '@rails/ujs'; import * as ActiveStorage from '@rails/activestorage'; import '@rails/actiontext'; import 'whatwg-fetch'; // window.fetch polyfill -import Chartkick from 'chartkick'; -import Highcharts from 'highcharts'; import ReactRailsUJS from 'react_ujs'; import '../shared/page-update-event'; @@ -37,7 +35,6 @@ import { motivationCancel, showImportJustificatif } from '../new_design/state-button'; -import { toggleChart } from '../new_design/toggle-chart'; import { replaceSemicolonByComma } from '../new_design/avis'; import { acceptEmailSuggestion, @@ -52,21 +49,18 @@ const DS = { showMotivation, motivationCancel, showImportJustificatif, - toggleChart, replaceSemicolonByComma, acceptEmailSuggestion, discardEmailSuggestionBox }; // Start Rails helpers -Chartkick.addAdapter(Highcharts); Rails.start(); Turbolinks.start(); ActiveStorage.start(); // Expose globals window.DS = window.DS || DS; -window.Chartkick = Chartkick; // (Both Rails redirects and ReactRailsUJS expect Turbolinks to be globally available) window.Turbolinks = Turbolinks; diff --git a/app/views/instructeurs/procedures/stats.html.haml b/app/views/instructeurs/procedures/stats.html.haml index 4346d99ff..ccb9b768c 100644 --- a/app/views/instructeurs/procedures/stats.html.haml +++ b/app/views/instructeurs/procedures/stats.html.haml @@ -6,6 +6,8 @@ 'Statistiques'] } .statistiques + = react_component('Chartkick') + %h1.new-h1= title .stat-cards - if @usual_traitement_time.present? diff --git a/app/views/stats/index.html.haml b/app/views/stats/index.html.haml index b69647565..49e6da724 100644 --- a/app/views/stats/index.html.haml +++ b/app/views/stats/index.html.haml @@ -1,6 +1,7 @@ - content_for(:title, 'Statistiques') .statistiques + = react_component('Chartkick') %h1.new-h1 Statistiques @@ -55,9 +56,9 @@ .stat-card.stat-card-half.pull-left %ul.segmented-control.pull-right - %li.segmented-control-item.segmented-control-item-active{ :onclick => "DS.toggleChart(event, '.monthly-procedures-chart');" } + %li.segmented-control-item.segmented-control-item-active{ data: { 'toggle-chart': '.monthly-procedures-chart' } } Par mois - %li.segmented-control-item{ :onclick => "DS.toggleChart(event, '.cumulative-procedures-chart');" } + %li.segmented-control-item{ data: { 'toggle-chart': '.cumulative-procedures-chart' } } Cumul %span.stat-card-title.pull-left Démarches dématérialisées .clearfix @@ -70,9 +71,9 @@ .stat-card.stat-card-half.pull-left %ul.segmented-control.pull-right - %li.segmented-control-item.segmented-control-item-active{ :onclick => "DS.toggleChart(event, '.monthly-dossiers-chart');" } + %li.segmented-control-item.segmented-control-item-active{ data: { 'toggle-chart': '.monthly-dossiers-chart' } } Par mois - %li.segmented-control-item{ :onclick => "DS.toggleChart(event, '.cumulative-dossiers-chart');" } + %li.segmented-control-item{ data: { 'toggle-chart': '.cumulative-dossiers-chart' } } Cumul %span.stat-card-title.pull-left Dossiers déposés .clearfix diff --git a/package.json b/package.json index 01656a298..3c5f81061 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ "debounce": "^1.2.0", "dom4": "^2.1.5", "email-butler": "^1.0.13", - "highcharts": "^6.1.2", + "highcharts": "^8.0.0", "intersection-observer": "^0.7.0", "jquery": "^3.4.1", "leaflet": "^1.6.0", diff --git a/yarn.lock b/yarn.lock index 501bc0a24..43276e44d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4426,10 +4426,10 @@ hex-color-regex@^1.1.0: resolved "https://registry.yarnpkg.com/hex-color-regex/-/hex-color-regex-1.1.0.tgz#4c06fccb4602fe2602b3c93df82d7e7dbf1a8a8e" integrity sha512-l9sfDFsuqtOqKDsQdqrMRk0U85RZc0RtOR9yPI7mRVOa4FsR/BVnZ0shmQRM96Ji99kYZP/7hn1cedc1+ApsTQ== -highcharts@^6.1.2: - version "6.2.0" - resolved "https://registry.yarnpkg.com/highcharts/-/highcharts-6.2.0.tgz#2a6d04652eb43c66f462ca7e2d2808f1f2782b61" - integrity sha512-A4E89MA+kto8giic7zyLU6ZxfXnVeCUlKOyzFsah3+n4BROx4bgonl92KIBtwLud/mIWir8ahqhuhe2by9LakQ== +highcharts@^8.0.0: + version "8.0.0" + resolved "https://registry.yarnpkg.com/highcharts/-/highcharts-8.0.0.tgz#fcf77a511d6ea477b9d8447afd9dedbfc85e2727" + integrity sha512-jRKLiP0i29zKSEgMDASyrfpivrM3e8V8yTv8YUZtRzB2mXR+hsBNHWZw2hNRZVxqq1QCVmW/9Z/BLFvYlJELqA== highlight.js@~9.12.0: version "9.12.0" From 662ef451a94b7fd5e7e2d940117c599ae4d5e4b4 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Feb 2020 09:12:48 +0100 Subject: [PATCH 08/14] Update app/views/stats/index.html.haml Co-Authored-By: Pierre de La Morinerie --- app/views/stats/index.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/views/stats/index.html.haml b/app/views/stats/index.html.haml index 49e6da724..8e0167a84 100644 --- a/app/views/stats/index.html.haml +++ b/app/views/stats/index.html.haml @@ -1,6 +1,8 @@ - content_for(:title, 'Statistiques') .statistiques + -# Load Chartkick lazily, by using our React lazy-loader. + -# (Chartkick itself doesn't use React though) = react_component('Chartkick') %h1.new-h1 Statistiques From ee1a930693e7f789e92495f4f8f2557cb50aba84 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Tue, 25 Feb 2020 09:12:58 +0100 Subject: [PATCH 09/14] Update app/views/instructeurs/procedures/stats.html.haml Co-Authored-By: Pierre de La Morinerie --- app/views/instructeurs/procedures/stats.html.haml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/views/instructeurs/procedures/stats.html.haml b/app/views/instructeurs/procedures/stats.html.haml index ccb9b768c..99a47afb6 100644 --- a/app/views/instructeurs/procedures/stats.html.haml +++ b/app/views/instructeurs/procedures/stats.html.haml @@ -6,6 +6,8 @@ 'Statistiques'] } .statistiques + -# Load Chartkick lazily, by using our React lazy-loader. + -# (Chartkick itself doesn't use React though) = react_component('Chartkick') %h1.new-h1= title @@ -31,4 +33,3 @@ .chart - colors = %w(#C3D9FF #0069CC #1C7EC9) # from _colors.scss = pie_chart @termines_states, colors: colors - From 98a37c9e9551cd5eace604ea760c3e919f20e725 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 25 Feb 2020 09:25:13 +0000 Subject: [PATCH 10/14] build(deps): bump nokogiri from 1.10.7 to 1.10.8 Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.7 to 1.10.8. - [Release notes](https://github.com/sparklemotion/nokogiri/releases) - [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md) - [Commits](https://github.com/sparklemotion/nokogiri/compare/v1.10.7...v1.10.8) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 33c623920..1c4931188 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -381,7 +381,7 @@ GEM nenv (0.3.0) netrc (0.11.0) nio4r (2.5.2) - nokogiri (1.10.7) + nokogiri (1.10.8) mini_portile2 (~> 2.4.0) notiffany (0.1.1) nenv (~> 0.1) From be66a8986ce84c49d2330af7c9f271853c93789e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 20 Feb 2020 13:25:32 +0100 Subject: [PATCH 11/14] Activate champ integer number on all the new procedures --- app/helpers/procedure_helper.rb | 54 ++------------------ app/models/type_de_champ.rb | 56 +++++++++++++++++++-- spec/factories/procedure.rb | 8 +++ spec/models/type_de_champ_shared_example.rb | 21 ++++++++ 4 files changed, 85 insertions(+), 54 deletions(-) diff --git a/app/helpers/procedure_helper.rb b/app/helpers/procedure_helper.rb index 135582b43..a6bb001de 100644 --- a/app/helpers/procedure_helper.rb +++ b/app/helpers/procedure_helper.rb @@ -21,8 +21,8 @@ module ProcedureHelper def types_de_champ_data(procedure) { isAnnotation: false, - typeDeChampsTypes: types_de_champ_types, - typeDeChamps: types_de_champ_as_json(procedure.types_de_champ), + typeDeChampsTypes: TypeDeChamp.type_de_champ_types_for(procedure, current_user), + typeDeChamps: procedure.types_de_champ.as_json_for_editor, baseUrl: procedure_types_de_champ_path(procedure), directUploadUrl: rails_direct_uploads_url } @@ -31,58 +31,12 @@ module ProcedureHelper def types_de_champ_private_data(procedure) { isAnnotation: true, - typeDeChampsTypes: types_de_champ_types, - typeDeChamps: types_de_champ_as_json(procedure.types_de_champ_private), + typeDeChampsTypes: TypeDeChamp.type_de_champ_types_for(procedure, current_user), + typeDeChamps: procedure.types_de_champ_private.as_json_for_editor, baseUrl: procedure_types_de_champ_path(procedure), directUploadUrl: rails_direct_uploads_url } end private - - TOGGLES = { - TypeDeChamp.type_champs.fetch(:integer_number) => :administrateur_champ_integer_number - } - - def types_de_champ_types - types_de_champ_types = TypeDeChamp.type_de_champs_list_fr - - types_de_champ_types.select! do |tdc| - toggle = TOGGLES[tdc.last] - toggle.blank? || feature_enabled?(toggle) - end - - types_de_champ_types - end - - TYPES_DE_CHAMP_BASE = { - except: [ - :created_at, - :options, - :order_place, - :parent_id, - :private, - :procedure_id, - :stable_id, - :type, - :updated_at - ], - methods: [ - :cadastres, - :drop_down_list_value, - :parcelles_agricoles, - :piece_justificative_template_filename, - :piece_justificative_template_url, - :quartiers_prioritaires - ] - } - TYPES_DE_CHAMP = TYPES_DE_CHAMP_BASE - .merge(include: { types_de_champ: TYPES_DE_CHAMP_BASE }) - - def types_de_champ_as_json(types_de_champ) - types_de_champ.includes(:drop_down_list, - piece_justificative_template_attachment: :blob, - types_de_champ: [:drop_down_list, piece_justificative_template_attachment: :blob]) - .as_json(TYPES_DE_CHAMP) - end end diff --git a/app/models/type_de_champ.rb b/app/models/type_de_champ.rb index 280e957d1..ca26be603 100644 --- a/app/models/type_de_champ.rb +++ b/app/models/type_de_champ.rb @@ -115,10 +115,6 @@ class TypeDeChamp < ApplicationRecord dynamic_type.build_champ end - def self.type_de_champs_list_fr - type_champs.map { |champ| [I18n.t("activerecord.attributes.type_de_champ.type_champs.#{champ.last}"), champ.first] } - end - def check_mandatory if non_fillable? self.mandatory = false @@ -162,6 +158,10 @@ class TypeDeChamp < ApplicationRecord type_champ == TypeDeChamp.type_champs.fetch(:dossier_link) end + def legacy_number? + type_champ == TypeDeChamp.type_champs.fetch(:number) + end + def public? !private? end @@ -194,6 +194,54 @@ class TypeDeChamp < ApplicationRecord GraphQL::Schema::UniqueWithinType.encode('Champ', stable_id) end + FEATURE_FLAGS = {} + + def self.type_de_champ_types_for(procedure, user) + has_legacy_number = (procedure.types_de_champ + procedure.types_de_champ_private).any?(&:legacy_number?) + + type_champs.map do |type_champ| + [I18n.t("activerecord.attributes.type_de_champ.type_champs.#{type_champ.last}"), type_champ.first] + end.filter do |tdc| + if tdc.last == TypeDeChamp.type_champs.fetch(:number) + has_legacy_number + else + feature_name = FEATURE_FLAGS[tdc.last] + feature_name.blank? || Flipper.enabled?(feature_name, user) + end + end + end + + TYPES_DE_CHAMP_BASE = { + except: [ + :created_at, + :options, + :order_place, + :parent_id, + :private, + :procedure_id, + :stable_id, + :type, + :updated_at + ], + methods: [ + :cadastres, + :drop_down_list_value, + :parcelles_agricoles, + :piece_justificative_template_filename, + :piece_justificative_template_url, + :quartiers_prioritaires + ] + } + TYPES_DE_CHAMP = TYPES_DE_CHAMP_BASE + .merge(include: { types_de_champ: TYPES_DE_CHAMP_BASE }) + + def self.as_json_for_editor + includes(:drop_down_list, + piece_justificative_template_attachment: :blob, + types_de_champ: [:drop_down_list, piece_justificative_template_attachment: :blob]) + .as_json(TYPES_DE_CHAMP) + end + private def set_default_drop_down_list diff --git a/spec/factories/procedure.rb b/spec/factories/procedure.rb index 9385382c1..32c4a6586 100644 --- a/spec/factories/procedure.rb +++ b/spec/factories/procedure.rb @@ -162,6 +162,14 @@ FactoryBot.define do end end + trait :with_number do + after(:build) do |procedure, _evaluator| + type_de_champ = create(:type_de_champ_number) + + procedure.types_de_champ << type_de_champ + end + end + trait :published do after(:build) do |procedure, _evaluator| procedure.path = generate(:published_path) diff --git a/spec/models/type_de_champ_shared_example.rb b/spec/models/type_de_champ_shared_example.rb index a710b4ef0..804ac127a 100644 --- a/spec/models/type_de_champ_shared_example.rb +++ b/spec/models/type_de_champ_shared_example.rb @@ -166,4 +166,25 @@ shared_examples 'type_de_champ_spec' do expect(messages.last.starts_with?("La liste doit commencer par")).to be_truthy end end + + describe '#type_de_champ_types_for' do + let(:procedure) { create(:procedure) } + let(:user) { create(:user) } + + context 'when procedure without legacy "number"' do + it 'should have "nombre decimal" instead of "nombre"' do + expect(TypeDeChamp.type_de_champ_types_for(procedure, user).find { |tdc| tdc.last == TypeDeChamp.type_champs.fetch(:number) }).to be_nil + expect(TypeDeChamp.type_de_champ_types_for(procedure, user).find { |tdc| tdc.last == TypeDeChamp.type_champs.fetch(:decimal_number) }).not_to be_nil + end + end + + context 'when procedure with legacy "number"' do + let(:procedure) { create(:procedure, :with_number) } + + it 'should have "nombre decimal" and "nombre"' do + expect(TypeDeChamp.type_de_champ_types_for(procedure, user).find { |tdc| tdc.last == TypeDeChamp.type_champs.fetch(:number) }).not_to be_nil + expect(TypeDeChamp.type_de_champ_types_for(procedure, user).find { |tdc| tdc.last == TypeDeChamp.type_champs.fetch(:decimal_number) }).not_to be_nil + end + end + end end From dcc81d42dd1ff66631d217bd76a3ddfee8050e97 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 13 Feb 2020 16:14:30 +0100 Subject: [PATCH 12/14] specs: ensure the procedure-less sign-up is tested --- spec/features/users/sign_up_spec.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 69fbcb78f..4a64fc382 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -5,16 +5,15 @@ feature 'Signing up:' do let(:user_password) { 'démarches-simplifiées-pwd' } let(:procedure) { create :simple_procedure, :with_service } - scenario 'a new user can sign-up' do - visit commencer_path(path: procedure.path) - click_on 'Créer un compte demarches-simplifiees.fr' + scenario 'a new user can sign-up from scratch' do + visit new_user_registration_path sign_up_with user_email, user_password expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}" click_confirmation_link_for user_email expect(page).to have_content 'Votre compte a été activé' - expect(page).to have_current_path commencer_path(path: procedure.path) + expect(page).to have_current_path dossiers_path end context 'when the user makes a typo in their email address' do From 10c940c1889017643f09f5582402e4ed642856ba Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 13 Feb 2020 16:14:52 +0100 Subject: [PATCH 13/14] specs: small cosmetic cleanups in sign-up specs --- spec/features/users/sign_up_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 4a64fc382..2775a4faa 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -67,11 +67,9 @@ feature 'Signing up:' do context 'when visiting a procedure' do let(:procedure) { create :simple_procedure, :with_service } - before do - visit commencer_path(path: procedure.path) - end - scenario 'a new user can sign-up and fill the procedure' do + visit commencer_path(path: procedure.path) + click_on 'Créer un compte' expect(page).to have_current_path new_user_registration_path expect(page).to have_procedure_description(procedure) @@ -102,7 +100,7 @@ feature 'Signing up:' do sign_up_with user_email, user_password # The same page than for initial sign-ups is displayed, to avoid leaking informations - # about the accound existence. + # about the account existence. expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}" # The confirmation email is sent again @@ -130,8 +128,8 @@ feature 'Signing up:' do warning_email = open_email(user_email) expect(warning_email.body).to have_text('Votre compte existe déjà') - # When clicking the main button, the user has a link to directly sign-in - # for the procedure they were initially starting + # When clicking the main button, the user is redirected directly to + # the sign-in page for the procedure they were initially starting. click_procedure_sign_in_link_for user_email expect(page).to have_current_path new_user_session_path From 6664965961954d1cd9a61467e3814aa6a223d614 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 24 Feb 2020 10:55:29 +0000 Subject: [PATCH 14/14] mailers: add procedure context to the confirmation link This allows to redirect the user to the procedure they signed up for even when the browser session is not available (like if they changed of browser). Fix #4738 --- app/controllers/users/confirmations_controller.rb | 10 ++++++++++ app/controllers/users/registrations_controller.rb | 6 ++++++ app/mailers/devise_user_mailer.rb | 1 + app/models/current_confirmation.rb | 3 +++ .../confirmation_instructions.html.haml | 3 ++- spec/features/users/sign_up_spec.rb | 12 +++++++++++- spec/mailers/previews/devise_user_mailer_preview.rb | 9 +++++++++ spec/support/feature_helpers.rb | 11 ++++++++--- 8 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 app/models/current_confirmation.rb diff --git a/app/controllers/users/confirmations_controller.rb b/app/controllers/users/confirmations_controller.rb index b66d836be..ea3a1c37b 100644 --- a/app/controllers/users/confirmations_controller.rb +++ b/app/controllers/users/confirmations_controller.rb @@ -42,9 +42,19 @@ class Users::ConfirmationsController < Devise::ConfirmationsController if sign_in_after_confirmation?(resource) resource.remember_me = true sign_in(resource) + end + + if procedure_from_params + commencer_path(path: procedure_from_params.path) + elsif signed_in? + # Will try to use `stored_location_for` to find a path after_sign_in_path_for(resource_name) else super(resource_name, resource) end end + + def procedure_from_params + params[:procedure_id] && Procedure.find_by(id: params[:procedure_id]) + end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 7f513dba5..557050554 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -21,6 +21,12 @@ class Users::RegistrationsController < Devise::RegistrationsController # POST /resource def create + # We may need the confirmation mailer to access the current procedure. + # But there's no easy way to pass an argument to the mailer through + # all Devise code. + # So instead we use a per-request global variable. + CurrentConfirmation.procedure_after_confirmation = @procedure + # Handle existing user trying to sign up again existing_user = User.find_by(email: params[:user][:email]) if existing_user.present? diff --git a/app/mailers/devise_user_mailer.rb b/app/mailers/devise_user_mailer.rb index de07cc1b7..eab62ff29 100644 --- a/app/mailers/devise_user_mailer.rb +++ b/app/mailers/devise_user_mailer.rb @@ -15,6 +15,7 @@ class DeviseUserMailer < Devise::Mailer def confirmation_instructions(record, token, opts = {}) opts[:from] = NO_REPLY_EMAIL + @procedure = CurrentConfirmation.procedure_after_confirmation || nil super end end diff --git a/app/models/current_confirmation.rb b/app/models/current_confirmation.rb new file mode 100644 index 000000000..c25314d7b --- /dev/null +++ b/app/models/current_confirmation.rb @@ -0,0 +1,3 @@ +class CurrentConfirmation < ActiveSupport::CurrentAttributes + attribute :procedure_after_confirmation +end diff --git a/app/views/devise_mailer/confirmation_instructions.html.haml b/app/views/devise_mailer/confirmation_instructions.html.haml index 08ed08de9..aa40e3834 100644 --- a/app/views/devise_mailer/confirmation_instructions.html.haml +++ b/app/views/devise_mailer/confirmation_instructions.html.haml @@ -7,7 +7,8 @@ %p Pour activer votre compte sur demarches-simplifiees.fr, veuillez cliquer sur le lien suivant : - = link_to(confirmation_url(@user, confirmation_token: @token), confirmation_url(@user, confirmation_token: @token)) + - link = confirmation_url(@user, confirmation_token: @token, procedure_id: @procedure&.id) + = link_to(link, link) - else - content_for(:title, "Changement d'adresse email") diff --git a/spec/features/users/sign_up_spec.rb b/spec/features/users/sign_up_spec.rb index 2775a4faa..4fb8fd0a7 100644 --- a/spec/features/users/sign_up_spec.rb +++ b/spec/features/users/sign_up_spec.rb @@ -77,8 +77,10 @@ feature 'Signing up:' do sign_up_with user_email, user_password expect(page).to have_content "nous avons besoin de vérifier votre adresse #{user_email}" - click_confirmation_link_for user_email + click_confirmation_link_for(user_email, in_another_browser: true) + # After confirmation, the user is redirected to the procedure they were initially starting + # (even when confirming the account in another browser). expect(page).to have_current_path(commencer_path(path: procedure.path)) expect(page).to have_content 'Votre compte a été activé' click_on 'Commencer la démarche' @@ -106,6 +108,14 @@ feature 'Signing up:' do # The confirmation email is sent again confirmation_email = open_email(user_email) expect(confirmation_email.body).to have_text('Pour activer votre compte') + + click_confirmation_link_for(user_email, in_another_browser: true) + + # After confirmation, the user is redirected to the procedure they were initially starting + # (even when confirming the account in another browser). + expect(page).to have_current_path(commencer_path(path: procedure.path)) + expect(page).to have_content 'Votre compte a été activé' + expect(page).to have_content 'Commencer la démarche' end end diff --git a/spec/mailers/previews/devise_user_mailer_preview.rb b/spec/mailers/previews/devise_user_mailer_preview.rb index 6aea003ec..e79fc17fc 100644 --- a/spec/mailers/previews/devise_user_mailer_preview.rb +++ b/spec/mailers/previews/devise_user_mailer_preview.rb @@ -3,6 +3,11 @@ class DeviseUserMailerPreview < ActionMailer::Preview DeviseUserMailer.confirmation_instructions(user, "faketoken", {}) end + def confirmation_instructions___with_procedure + CurrentConfirmation.procedure_after_confirmation = procedure + DeviseUserMailer.confirmation_instructions(user, "faketoken", {}) + end + def reset_password_instructions DeviseUserMailer.reset_password_instructions(user, "faketoken", {}) end @@ -12,4 +17,8 @@ class DeviseUserMailerPreview < ActionMailer::Preview def user User.new(id: 10, email: "usager@example.com") end + + def procedure + Procedure.new(id: 20, libelle: 'Dotation d’Équipement des Territoires Ruraux - Exercice 2019', path: 'dotation-etr') + end end diff --git a/spec/support/feature_helpers.rb b/spec/support/feature_helpers.rb index 0cbc98c68..b1cb303eb 100644 --- a/spec/support/feature_helpers.rb +++ b/spec/support/feature_helpers.rb @@ -48,11 +48,16 @@ module FeatureHelpers end end - def click_confirmation_link_for(email) + def click_confirmation_link_for(email, in_another_browser: false) confirmation_email = open_email(email) - token_params = confirmation_email.body.match(/confirmation_token=[^"]+/) + confirmation_link = confirmation_email.body.match(/href="[^"]*(\/users\/confirmation[^"]*)"/)[1] - visit "/users/confirmation?#{token_params}" + if in_another_browser + # Simulate the user opening the link in another browser, thus loosing the session cookie + Capybara.reset_session! + end + + visit confirmation_link end def click_procedure_sign_in_link_for(email)