From 61660b1b3b2b48e2eedf4324e7ff983dbd9577de Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Thu, 9 Apr 2020 14:45:57 +0000 Subject: [PATCH 1/8] javascript: remove unused jQuery.active bookkeeping --- app/javascript/shared/rails-ujs-fix.js | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/app/javascript/shared/rails-ujs-fix.js b/app/javascript/shared/rails-ujs-fix.js index acff94246..44c7f6c4c 100644 --- a/app/javascript/shared/rails-ujs-fix.js +++ b/app/javascript/shared/rails-ujs-fix.js @@ -1,20 +1,5 @@ import Rails from '@rails/ujs'; import jQuery from 'jquery'; -import { delegate } from '@utils'; - -// We use `jQuery.active` in our capybara suit to wait for ajax requests. -// Newer jQuery-less version of rails-ujs is breaking it. -// We have to set `ajax:complete` listener on the same element as the one -// we catch ajax:send on as by the end of the request -// the old element may be removed from DOM. -delegate('ajax:send', '[data-remote]', ({ target }) => { - let callback = () => { - jQuery.active--; - target.removeEventListener('ajax:complete', callback); - }; - target.addEventListener('ajax:complete', callback); - jQuery.active++; -}); // `smart_listing` gem is overriding `$.rails.href` method. When using newer // jQuery-less version of rails-ujs it breaks. From 7a8fd3c679a02965a4667e295e39e847623beb4e Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 9 Apr 2020 12:41:35 +0200 Subject: [PATCH 2/8] Use graphql playground instead of graphiql --- Gemfile | 2 +- Gemfile.lock | 7 ++- config/initializers/graphiql.rb | 81 --------------------------------- config/initializers/graphql.rb | 7 +++ config/routes.rb | 2 +- 5 files changed, 12 insertions(+), 87 deletions(-) delete mode 100644 config/initializers/graphiql.rb diff --git a/Gemfile b/Gemfile index 55ba93826..14407ddc5 100644 --- a/Gemfile +++ b/Gemfile @@ -31,10 +31,10 @@ gem 'flipper-ui' gem 'fugit' gem 'geocoder' gem 'gon' -gem 'graphiql-rails' gem 'graphql' gem 'graphql-batch' gem 'graphql-rails_logger' +gem 'graphql_playground-rails' gem 'groupdate' gem 'haml-rails' gem 'hashie' diff --git a/Gemfile.lock b/Gemfile.lock index 224ba4f58..7e80e2d2b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -248,9 +248,6 @@ GEM i18n (>= 0.7) multi_json request_store (>= 1.0) - graphiql-rails (1.7.0) - railties - sprockets-rails graphql (1.10.6) graphql-batch (0.4.2) graphql (>= 1.3, < 2) @@ -264,6 +261,8 @@ GEM bundler (>= 1.14) graphql (~> 1.10) thor (>= 0.19, < 2.0) + graphql_playground-rails (2.1.0) + rails (>= 5.1.0) groupdate (5.0.0) activesupport (>= 5) guard (2.15.0) @@ -755,11 +754,11 @@ DEPENDENCIES fugit geocoder gon - graphiql-rails graphql graphql-batch graphql-rails_logger graphql-schema_comparator + graphql_playground-rails groupdate guard guard-livereload diff --git a/config/initializers/graphiql.rb b/config/initializers/graphiql.rb deleted file mode 100644 index b67ca0c50..000000000 --- a/config/initializers/graphiql.rb +++ /dev/null @@ -1,81 +0,0 @@ -DEFAULT_QUERY = "# La documentation officielle de la spécification (Anglais) : https://graphql.org/ -# Une introduction aux concepts et raisons d'être de GraphQL (Français) : https://blog.octo.com/graphql-et-pourquoi-faire/ -# Le schema GraphQL de demarches-simplifiees.fr : https://demarches-simplifiees-graphql.netlify.com -# Le endpoint GraphQL de demarches-simplifiees.fr : https://www.demarches-simplifiees.fr/api/v2/graphql - -query getDemarche($demarcheNumber: Int!) { - demarche(number: $demarcheNumber) { - id - number - title - champDescriptors { - id - type - label - } - dossiers(first: 3) { - nodes { - id - number - datePassageEnConstruction - datePassageEnInstruction - dateTraitement - usager { - email - } - champs { - id - label - ... on TextChamp { - value - } - ... on DecimalNumberChamp { - value - } - ... on IntegerNumberChamp { - value - } - ... on CheckboxChamp { - value - } - ... on DateChamp { - value - } - ... on DossierLinkChamp { - dossier { - id - } - } - ... on MultipleDropDownListChamp { - values - } - ... on LinkedDropDownListChamp { - primaryValue - secondaryValue - } - ... on PieceJustificativeChamp { - file { - url - } - } - ... on CarteChamp { - geoAreas { - source - geometry { - type - coordinates - } - } - } - } - } - pageInfo { - hasNextPage - endCursor - } - } - } -}" - -GraphiQL::Rails.config.initial_query = DEFAULT_QUERY -GraphiQL::Rails.config.title = 'demarches-simplifiees.fr' diff --git a/config/initializers/graphql.rb b/config/initializers/graphql.rb index e669da38b..dbf0da270 100644 --- a/config/initializers/graphql.rb +++ b/config/initializers/graphql.rb @@ -3,3 +3,10 @@ GraphQL::RailsLogger.configure do |config| 'API::V2::GraphqlController' => ['execute'] } end + +GraphqlPlayground::Rails.configure do |config| + config.title = "demarches-simplifiees.fr" + config.settings = { + "schema.polling.enable": false + } +end diff --git a/config/routes.rb b/config/routes.rb index 35f560b5c..456bb92fd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -224,7 +224,7 @@ Rails.application.routes.draw do # authenticated :user, lambda { |user| user.administrateur_id && Flipper.enabled?(:administrateur_graphql, user) } do - mount GraphiQL::Rails::Engine, at: "/graphql", graphql_path: "/api/v2/graphql", via: :get + mount GraphqlPlayground::Rails::Engine, at: "/graphql", graphql_path: "/api/v2/graphql" end namespace :api do From 6df927a46f0abee3a60a8a9b0ad2ac76e79d0224 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 9 Apr 2020 11:31:45 +0200 Subject: [PATCH 3/8] nom du demandeur dans user::dossiers#index si >=2 dossiers --- app/helpers/dossier_helper.rb | 14 ++++++++++++++ app/views/users/dossiers/index.html.haml | 5 +++++ 2 files changed, 19 insertions(+) diff --git a/app/helpers/dossier_helper.rb b/app/helpers/dossier_helper.rb index 1341eeaaa..ab931bb24 100644 --- a/app/helpers/dossier_helper.rb +++ b/app/helpers/dossier_helper.rb @@ -1,4 +1,6 @@ module DossierHelper + include EtablissementHelper + def button_or_label_class(dossier) if dossier.accepte? 'accepted' @@ -101,6 +103,18 @@ module DossierHelper content_tag(:span, status_text, class: "label #{status_class} ") end + def demandeur_dossier(dossier) + if dossier.procedure.for_individual? + "#{dossier&.individual&.nom} #{dossier&.individual&.prenom}" + else + if dossier.etablissement.present? + raison_sociale_or_name(dossier.etablissement) + else + "" + end + end + end + private def dinum_instance? diff --git a/app/views/users/dossiers/index.html.haml b/app/views/users/dossiers/index.html.haml index 81d1709ec..0c73c103e 100644 --- a/app/views/users/dossiers/index.html.haml +++ b/app/views/users/dossiers/index.html.haml @@ -32,6 +32,8 @@ %th.notification-col %th.number-col Nº dossier %th Démarche + - if @dossiers.count > 1 + %th Demandeur %th.status-col Statut %th.updated-at-col Mis à jour %th @@ -47,6 +49,9 @@ %td = link_to(url_for_dossier(dossier), class: 'cell-link') do = procedure_libelle(dossier.procedure) + - if @dossiers.count > 1 + %td.number-col + = demandeur_dossier(dossier) %td.status-col = link_to(url_for_dossier(dossier), class: 'cell-link') do = status_badge(dossier.state) From 7335500be4214dbab0ef20283364bc950f79954b Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Thu, 9 Apr 2020 14:09:39 +0200 Subject: [PATCH 4/8] tests for dossier_helper#demandeur_dossier --- spec/helpers/dossier_helper_spec.rb | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/spec/helpers/dossier_helper_spec.rb b/spec/helpers/dossier_helper_spec.rb index 0bf33c397..7e90e8807 100644 --- a/spec/helpers/dossier_helper_spec.rb +++ b/spec/helpers/dossier_helper_spec.rb @@ -38,6 +38,40 @@ RSpec.describe DossierHelper, type: :helper do end end + describe ".demandeur_dossier" do + subject { demandeur_dossier(dossier) } + + let(:individual) { create(:individual) } + let(:etablissement) { create(:etablissement) } + let(:dossier) { create(:dossier, procedure: procedure, individual: individual, etablissement: etablissement) } + + context "when the dossier is for an individual" do + let(:procedure) { create(:simple_procedure, :for_individual) } + + context "when the individual is not provided" do + let(:individual) { nil } + it { is_expected.to be_blank } + end + + context "when the individual has name information" do + it { is_expected.to eq "#{individual.nom} #{individual.prenom}" } + end + end + + context "when the dossier is for a company" do + let(:procedure) { create(:procedure, for_individual: false) } + + context "when the company is not provided" do + let(:etablissement) { nil } + it { is_expected.to be_blank } + end + + context "when the company has name information" do + it { is_expected.to eq raison_sociale_or_name(etablissement) } + end + end + end + describe ".dossier_submission_is_closed?" do let(:dossier) { create(:dossier, state: state) } let(:state) { Dossier.states.fetch(:brouillon) } From 8c2f589cbf17796b5657ffa2156f236ed761b8f5 Mon Sep 17 00:00:00 2001 From: clemkeirua Date: Fri, 10 Apr 2020 17:41:51 +0200 Subject: [PATCH 5/8] fix admin deletion of empty service with archived procedures --- app/models/administrateur.rb | 5 +++-- spec/models/administrateur_spec.rb | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/app/models/administrateur.rb b/app/models/administrateur.rb index 0b982aabf..474ae1982 100644 --- a/app/models/administrateur.rb +++ b/app/models/administrateur.rb @@ -81,13 +81,14 @@ class Administrateur < ApplicationRecord fail "Impossible de supprimer cet administrateur car il a des démarches où il est le seul administrateur" end - procedures.each do |procedure| + procedures.with_discarded.each do |procedure| next_administrateur = procedure.administrateurs.where.not(id: self.id).first procedure.service.update(administrateur: next_administrateur) end services.each do |service| - service.destroy unless service.procedures.any? + # We can't destroy a service if it has procedures, even if those procedures are archived + service.destroy unless service.procedures.with_discarded.any? end destroy diff --git a/spec/models/administrateur_spec.rb b/spec/models/administrateur_spec.rb index c1e4e3320..e0ac8f044 100644 --- a/spec/models/administrateur_spec.rb +++ b/spec/models/administrateur_spec.rb @@ -65,6 +65,15 @@ describe Administrateur, type: :model do expect(Service.find_by(id: service_without_procedure.id)).to be_nil expect(Administrateur.find_by(id: administrateur.id)).to be_nil end + + it "does not delete service if associated to an archived procedure" do + service.update(administrateur: administrateur) + procedure.discard! + administrateur.delete_and_transfer_services + + expect(Service.find_by(id: procedure.service.id)).not_to be_nil + expect(Administrateur.find_by(id: administrateur.id)).to be_nil + end end # describe '#password_complexity' do From 52d2ace823ace22b2f18bea5fe86695142a82104 Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 9 Apr 2020 19:08:18 +0200 Subject: [PATCH 6/8] Remove quartiers prioritaires --- app/controllers/champs/carte_controller.rb | 8 ---- .../components/TypeDeChamp.js | 4 -- app/lib/api_carto/api.rb | 5 -- .../quartiers_prioritaires_adapter.rb | 15 ------ app/services/geojson_service.rb | 22 --------- .../champs/carte_controller_spec.rb | 8 ++-- .../new_administrateur/types_de_champ_spec.rb | 7 ++- spec/fixtures/files/api_carto/request_qp.json | 29 ----------- .../fixtures/files/api_carto/response_qp.json | 30 ------------ spec/lib/api_carto/api_spec.rb | 48 ------------------- .../quartiers_prioritaires_adapter_spec.rb | 36 -------------- spec/services/geojson_service_spec.rb | 18 ------- 12 files changed, 7 insertions(+), 223 deletions(-) delete mode 100644 app/lib/api_carto/quartiers_prioritaires_adapter.rb delete mode 100644 spec/fixtures/files/api_carto/request_qp.json delete mode 100644 spec/fixtures/files/api_carto/response_qp.json delete mode 100644 spec/lib/api_carto/quartiers_prioritaires_adapter_spec.rb diff --git a/app/controllers/champs/carte_controller.rb b/app/controllers/champs/carte_controller.rb index 3fdd88b7b..4459ab781 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -39,14 +39,6 @@ class Champs::CarteController < ApplicationController end end - if @champ.quartiers_prioritaires? - quartiers_prioritaires = ApiCartoService.generate_qp(coordinates) - geo_areas += quartiers_prioritaires.map do |qp| - qp[:source] = GeoArea.sources.fetch(:quartier_prioritaire) - qp - end - end - selection_utilisateur = ApiCartoService.generate_selection_utilisateur(coordinates) selection_utilisateur[:source] = GeoArea.sources.fetch(:selection_utilisateur) geo_areas << selection_utilisateur diff --git a/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js b/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js index 5b4dc12fa..3787639e5 100644 --- a/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js +++ b/app/javascript/components/TypesDeChampEditor/components/TypeDeChamp.js @@ -136,10 +136,6 @@ const TypeDeChamp = sortableElement( url={typeDeChamp.piece_justificative_template_url} /> - /.*/, - :headers => { 'Content-Type' => 'application/json' }) - .to_return(status: status, body: body) - end - context 'when geojson is empty' do - let(:geojson) { '' } - let(:status) { 404 } - let(:body) { '' } - - it 'raises ApiCarto::API::ResourceNotFound' do - expect { subject }.to raise_error(ApiCarto::API::ResourceNotFound) - end - end - - context 'when request return 500' do - let(:geojson) { File.read('spec/fixtures/files/api_carto/request_qp.json') } - let(:status) { 500 } - let(:body) { 'toto' } - - it 'raises ApiCarto::API::ResourceNotFound' do - expect { subject }.to raise_error(ApiCarto::API::ResourceNotFound) - end - end - - context 'when geojson exist' do - let(:geojson) { File.read('spec/fixtures/files/api_carto/request_qp.json') } - let(:status) { 200 } - let(:body) { 'toto' } - - it 'returns response body' do - expect(subject).to eq(body) - end - - context 'when geojson is at format JSON' do - let(:geojson) { JSON.parse(File.read('spec/fixtures/files/api_carto/request_qp.json')) } - - it 'returns response body' do - expect(subject).to eq(body) - end - end - end - end - describe '.search_cadastre' do subject { described_class.search_cadastre(geojson) } diff --git a/spec/lib/api_carto/quartiers_prioritaires_adapter_spec.rb b/spec/lib/api_carto/quartiers_prioritaires_adapter_spec.rb deleted file mode 100644 index 4f1988bae..000000000 --- a/spec/lib/api_carto/quartiers_prioritaires_adapter_spec.rb +++ /dev/null @@ -1,36 +0,0 @@ -describe ApiCarto::QuartiersPrioritairesAdapter do - subject { described_class.new(coordinates).results } - - before do - stub_request(:post, "https://sandbox.geo.api.gouv.fr/apicarto/quartiers-prioritaires/search") - .with(:body => /.*/, - :headers => { 'Content-Type' => 'application/json' }) - .to_return(status: status, body: body) - end - - context 'coordinates are filled' do - let(:coordinates) { '[[2.252728, 43.27151][2.323223, 32.835332]]' } - let(:status) { 200 } - let(:body) { File.read('spec/fixtures/files/api_carto/response_qp.json') } - - it { expect(subject).to be_a_instance_of(Array) } - - context 'Attributes' do - let(:qp_code) { 'QP057019' } - - it { expect(subject.first[:code]).to eq(qp_code) } - it { expect(subject.first[:nom]).to eq('Hauts De Vallières') } - it { expect(subject.first[:commune]).to eq('Metz') } - - it { expect(subject.first[:geometry]).to eq({ :type => "MultiPolygon", :coordinates => [[[[6.2136923480551, 49.1342109827851], [6.21416055031881, 49.1338823553928]]]] }) } - end - end - - context 'coordinates are empty' do - let(:coordinates) { '' } - let(:status) { 404 } - let(:body) { '' } - - it { expect { subject }.to raise_error(ApiCarto::API::ResourceNotFound) } - end -end diff --git a/spec/services/geojson_service_spec.rb b/spec/services/geojson_service_spec.rb index bcd9fdea3..7a3bd7c80 100644 --- a/spec/services/geojson_service_spec.rb +++ b/spec/services/geojson_service_spec.rb @@ -9,24 +9,6 @@ describe GeojsonService do ] } - describe '.toGeoJsonPolygonForQp' do - subject { JSON.parse(described_class.to_json_polygon_for_qp coordinates) } - - describe 'coordinates are empty' do - let(:coordinates) { '' } - - it { expect(subject['geo']['type']).to eq('Polygon') } - it { expect(subject['geo']['coordinates']).to eq([coordinates]) } - end - - describe 'coordinates are informed' do - let(:coordinates) { good_coordinates } - - it { expect(subject['geo']['type']).to eq('Polygon') } - it { expect(subject['geo']['coordinates']).to eq([coordinates]) } - end - end - describe '.toGeoJsonPolygonForCadastre' do subject { JSON.parse(described_class.to_json_polygon_for_cadastre coordinates) } From 9e76135b275353e698d00cc02fea5424e1667bb2 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 14 Apr 2020 18:02:52 +0200 Subject: [PATCH 7/8] views: add identifiers to fooker links In Sentry, when an `ActionController::InvalidAuthenticityToken` error occurs, the breadcrumbs mention that before hitting the error, the user clicked on one of those links. Unfortunately we don't know which one. For debugging purposes, adding classes to the links should allow us to see which links users are navigating to. --- app/views/users/_general_footer_row.html.haml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/users/_general_footer_row.html.haml b/app/views/users/_general_footer_row.html.haml index 987c199e4..52e27bfe0 100644 --- a/app/views/users/_general_footer_row.html.haml +++ b/app/views/users/_general_footer_row.html.haml @@ -1,7 +1,7 @@ %ul.footer-row.footer-bottom-line.footer-site-links - %li>= link_to "Accessibilité", accessibilite_path - %li>= link_to "CGU", CGU_URL, target: "_blank", rel: "noopener noreferrer" - %li>= link_to "Mentions légales", MENTIONS_LEGALES_URL, target: "_blank", rel: "noopener noreferrer" - %li>= link_to 'Documentation', DOC_URL - %li>= contact_link "Contact technique", dossier_id: dossier&.id - %li>= link_to 'Aide', FAQ_URL + %li.footer-link-accessibilite>= link_to "Accessibilité", accessibilite_path + %li.footer-link-cgu>= link_to "CGU", CGU_URL, target: "_blank", rel: "noopener noreferrer" + %li.footer-link-mentions-legales>= link_to "Mentions légales", MENTIONS_LEGALES_URL, target: "_blank", rel: "noopener noreferrer" + %li.footer-link-doc>= link_to 'Documentation', DOC_URL + %li.footer-link-contact>= contact_link "Contact technique", dossier_id: dossier&.id + %li.footer-link-aide>= link_to 'Aide', FAQ_URL From 968e470684e3e2bfe51015fcb7a20d33038a64d7 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 14 Apr 2020 16:28:15 +0000 Subject: [PATCH 8/8] config: never cache rails-generated pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This instruct browsers to never cache content directly generated by the controllers. This includes HTML pages, JSON responses, PDF files, etc. This is because Some mobile browsers have a behaviour where, although they will delete the session cookie when the browser shutdowns, they will still serve a cached version of the page on relaunch. The CSRF token in the HTML is then mismatched with the CSRF token in the session cookie (because the session cookie has been cleared). This causes form submissions to fail with an "ActionController::InvalidAuthenticityToken" exception. To prevent this, tell browsers to never cache the HTML of a page. (This doesn’t affect assets files, which are still sent with the proper cache headers). See https://github.com/rails/rails/issues/21948 --- config/application.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/config/application.rb b/config/application.rb index be10d163e..2ecce58c2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -35,6 +35,18 @@ module TPS config.action_view.sanitized_allowed_tags = ActionView::Base.sanitized_allowed_tags + ['u'] + # Some mobile browsers have a behaviour where, although they will delete the session + # cookie when the browser shutdowns, they will still serve a cached version + # of the page on relaunch. + # The CSRF token in the HTML is then mismatched with the CSRF token in the session cookie + # (because the session cookie has been cleared). This causes form submissions to fail with + # a "ActionController::InvalidAuthenticityToken" exception. + # To prevent this, tell browsers to never cache the HTML of a page. + # (This doesn’t affect assets files, which are still sent with the proper cache headers). + # + # See https://github.com/rails/rails/issues/21948 + config.action_dispatch.default_headers['Cache-Control'] = 'no-store, no-cache' + config.to_prepare do # Make main application helpers available in administrate Administrate::ApplicationController.helper(TPS::Application.helpers)