Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ajoute un lien vers l'Espace Producteur dans le menu quand pertinent #3684

Merged
merged 14 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions apps/shared/lib/helpers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,4 @@ defmodule Helpers do
dates -> dates |> Enum.max(DateTime) |> DateTime.to_iso8601()
end
end

@spec admin?(map | nil) :: boolean
def admin?(%{} = user) do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supprime cette méthode elle faisait doublon avec ce qui était dans TransportWeb.Router

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user
|> Map.get("organizations", [])
|> Enum.any?(fn org -> org["slug"] == "equipe-transport-data-gouv-fr" end)
end

def admin?(nil), do: false
end
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ defmodule Transport.Jobs.DatasetNowOnNAPNotificationJob do
Phoenix.View.render_to_string(TransportWeb.EmailView, "dataset_now_on_nap.html",
dataset_url: TransportWeb.Router.Helpers.dataset_url(TransportWeb.Endpoint, :details, dataset.slug),
dataset_custom_title: dataset.custom_title,
contact_email_address: Application.get_env(:transport, :contact_email),
espace_producteur_url: TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur)
contact_email_address: Application.get_env(:transport, :contact_email)
)
)

Expand Down
1 change: 1 addition & 0 deletions apps/transport/lib/transport_web.ex
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ defmodule TransportWeb do
use TransportWeb.InputHelpers

import TransportWeb.Router.Helpers
import TransportWeb.Router, only: [is_transport_data_gouv_member?: 1]
import TransportWeb.ErrorHelpers
import TransportWeb.InputHelpers
import TransportWeb.Gettext
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Les changements importants dans la session sont dans ce fichier.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ defmodule TransportWeb.SessionController do
"""
use TransportWeb, :controller
alias Datagouvfr.Authentication
import Ecto.Query
require Logger

def new(conn, _) do
Expand Down Expand Up @@ -127,24 +128,45 @@ defmodule TransportWeb.SessionController do
end

def save_current_user(%Plug.Conn{} = conn, %{} = user_params) do
conn |> put_session(:current_user, user_params |> user_params_for_session())
conn |> put_session(:current_user, user_params_for_session(user_params))
end

thbar marked this conversation as resolved.
Show resolved Hide resolved
def user_params_for_session(%{} = params) do
params
# Remove the list of `organizations` from the final map: it's already stored in the database
# and maintained up-to-date by `Transport.Jobs.UpdateContactsJob`
# and it can be too big to be stored in a cookie
|> Map.delete("organizations")
# - `is_admin` is needed to check permissions
# - `is_producer` is used to get access to the "Espace producteur"
|> Map.merge(%{"is_producer" => is_producer?(params), "is_admin" => is_admin?(params)})
AntoineAugusti marked this conversation as resolved.
Show resolved Hide resolved
end

@doc """
iex> pan_org = %{"slug" => "equipe-transport-data-gouv-fr", "name" => "PAN"}
iex> other_org = %{"slug" => "foo-inc", "name" => "Foo Inc"}
iex> user_params_for_session(%{"foo" => "bar", "organizations" => [pan_org, other_org]})
%{"foo" => "bar", "organizations" => [pan_org]}
Are you a data producer?
You're a data producer if you're a member of an organization with an active dataset
on transport.data.gouv.fr.
This is set when you log in, we can refresh this field more often in the future.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

La durée de notre session est de 15 jours

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mis à jour dans l'Espace Producteur aussi b24229e

"""
def user_params_for_session(%{} = params) do
Map.put(
params,
"organizations",
Enum.filter(
params["organizations"],
&match?(%{"slug" => "equipe-transport-data-gouv-fr"}, &1)
)
)
def is_producer?(%{"organizations" => orgs}) do
AntoineAugusti marked this conversation as resolved.
Show resolved Hide resolved
org_ids = Enum.map(orgs, & &1["id"])

DB.Dataset.base_query() |> where([dataset: d], d.organization_id in ^org_ids) |> DB.Repo.exists?()
end

@doc """
Are you a transport.data.gouv.fr admin?
You're an admin if you're a member of the PAN organization on data.gouv.fr.

AntoineAugusti marked this conversation as resolved.
Show resolved Hide resolved
iex> is_admin?(%{"organizations" => [%{"slug" => "equipe-transport-data-gouv-fr"}, %{"slug" => "foo"}]})
true
iex> is_admin?(%{"organizations" => [%{"slug" => "foo"}]})
false
iex> is_admin?(%{"organizations" => []})
false
"""
def is_admin?(%{"organizations" => orgs}) do
Enum.any?(orgs, &(&1["slug"] == "equipe-transport-data-gouv-fr"))
end

defp get_redirect_path(%Plug.Conn{} = conn) do
Expand Down
9 changes: 4 additions & 5 deletions apps/transport/lib/transport_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ defmodule TransportWeb.Router do
end

defp assign_current_user(conn, _) do
# `current_user` is set by TransportWeb.SessionController.user_params_for_session/1
assign(conn, :current_user, get_session(conn, :current_user))
end

Expand Down Expand Up @@ -353,11 +354,9 @@ defmodule TransportWeb.Router do
end

# NOTE: method visibility set to public because we need to call the same logic from LiveView
def is_transport_data_gouv_member?(current_user) do
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ce changement impose aux membres de notre équipe de se déconnecter/reconnecter étant donné qu'on n'a pas l'attribut is_admin dans notre session actuellement. Si les membres de notre équipe ne font pas ça elles ne verront plus le lien vers le BO et si ils vont sur l'URL ils seront redirigés avec une erreur. Rien de dramatique donc.

Pas d'impact pour les autres.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amélioration sécurité à prévoir en lien avec:

En cas de vol de cookie, si je comprends bien (avant la PR ou après la PR d'ailleurs), le rôle restera encodé dans le cookie.

Il faudrait avoir un rafraîchissement systématique (ou plus frais en tout cas, ex: cache avec TTL si on veut éviter un appel d'API à chaque tour) sinon on se crée des problèmes (la suppression d'un compte sur data gouv ne protègera pas d'un vol de cookie admin).

current_user
|> Map.get("organizations", [])
|> Enum.any?(fn org -> org["slug"] == "equipe-transport-data-gouv-fr" end)
end
# `current_user` is set by TransportWeb.SessionController.user_params_for_session/1
def is_transport_data_gouv_member?(%{"is_admin" => true} = _current_user), do: true
def is_transport_data_gouv_member?(_), do: false

# Check that a secret key is passed in the URL in the `export_key` query parameter
defp check_export_secret_key(%Plug.Conn{params: params} = conn, _) do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<section :if={admin?(@conn.assigns[:current_user])} class="pt-48">
<section :if={is_transport_data_gouv_member?(@conn.assigns[:current_user])} class="pt-48">
<h2><%= dgettext("page-dataset-details", "Dataset scores") %></h2>
<div class="panel" id="scores-chart">
<div id="vega-vis"></div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<h1>
<%= @dataset.custom_title %>
</h1>
<%= if admin?(assigns[:current_user]) do %>
<%= if is_transport_data_gouv_member?(assigns[:current_user]) do %>
<i class="fa fa-external-link-alt"></i>
<%= link("backoffice", to: backoffice_page_path(@conn, :edit, @dataset.id)) %>
<%= render("_dataset_scores.html", dataset_scores: @dataset_scores, locale: locale) %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
<a href={dataset_path(@conn, :details, dataset.slug)}>
<%= dataset.custom_title %>
</a>
<%= if admin?(assigns[:current_user]) do %>
<%= if is_transport_data_gouv_member?(assigns[:current_user]) do %>
<span class="dataset-backoffice-link">
<i class="fa fa-external-link-alt"></i>
<%= link("backoffice", to: backoffice_page_path(@conn, :edit, dataset.id)) %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ Bonjour,

Votre jeu de données [<%= @dataset_custom_title %>](<%= @dataset_url %>) a bien été référencé sur le Point d’Accès National aux données de transport, [transport.data.gouv.fr](https://transport.data.gouv.fr).

Rendez-vous sur votre [Espace Producteur](<%= @espace_producteur_url %>) pour mettre à jour vos données ou vous inscrire à des notifications en cas de péremption, d’indisponibilité ou d’erreurs bloquantes sur votre jeu de données.
Rendez-vous sur votre [Espace Producteur](<%= espace_producteur_url(:dataset_now_on_nap) %>) pour mettre à jour vos données ou vous inscrire à des notifications en cas de péremption, d’indisponibilité ou d’erreurs bloquantes sur votre jeu de données.

Si vous avez des questions, n’hésitez pas à contacter notre équipe à l’adresse : <%= @contact_email_address %>.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Vous êtes inscrit à des notifications concernant les jeux de données suivants
</ul>
<% end %>

Les notifications vous permettent d’être alerté en cas d’expiration, d’indisponibilité et d’erreurs de vos données. Rendez-vous sur votre [Espace Producteur](<%= TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur) %>) pour les gérer de manière autonome.
Les notifications vous permettent d’être alerté en cas d’expiration, d’indisponibilité et d’erreurs de vos données. Rendez-vous sur votre [Espace Producteur](<%= espace_producteur_url(:periodic_reminder_producer_with_subscriptions) %>) pour les gérer de manière autonome.

<%= if @has_other_producers_subscribers do %>
Les autres personnes inscrites à ces notifications sont : <%= @other_producers_subscribers %>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Le saviez-vous ? Il est possible de vous inscrire à des notifications concernan

Les notifications vous permettent d’être alerté en cas d’expiration, d’indisponibilité et d’erreurs de vos données.

Pour vous inscrire, rien de plus simple : rendez-vous sur votre [Espace Producteur](<%= TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur) %>) dans le menu “Recevoir des notifications”.
Pour vous inscrire, rien de plus simple : rendez-vous sur votre [Espace Producteur](<%= espace_producteur_url(:periodic_reminder_producer_without_subscriptions) %>) dans le menu “Recevoir des notifications”.

Nous restons disponibles pour vous accompagner si besoin.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Il semble que vous ayez supprimé puis créé une nouvelle ressource : l’URL d

Pour les prochaines mises à jour, afin de garantir une URL stable, nous vous invitons à remplacer votre ressource obsolète par la nouvelle.

Pour cela, rendez-vous sur votre [Espace Producteur](<%= TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur) %>) à partir duquel vous pourrez procéder à ces mises à jour.
Pour cela, rendez-vous sur votre [Espace Producteur](<%= espace_producteur_url(:resource_unavailable_producer) %>) à partir duquel vous pourrez procéder à ces mises à jour.

Retrouvez la procédure pas à pas [sur notre documentation](https://doc.transport.data.gouv.fr/producteurs/mettre-a-jour-des-donnees).
<% else %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,14 @@
<% end %>
</div>
<div class="dropdown-content">
<%= if admin?(assigns[:current_user]) do %>
<%= if is_transport_data_gouv_member?(assigns[:current_user]) do %>
<%= link("Administration", to: "/backoffice") %>
<% end %>
<%= if Map.get(assigns[:current_user], "is_producer", false) do %>
<%= link(gettext("Producer space"),
to: page_path(@conn, :espace_producteur, utm_source: "menu_dropdown")
) %>
<% end %>
<a
class="navigation__link nagivation__link--logout"
href={session_path(@conn, :delete, redirect_path: current_path(@conn))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<%= if assigns[:current_user] do %>
<div class="panel-producteurs signed-in">
<h2><%= dgettext("page-producteurs", "Welcome!") %></h2>
<a class="button" href={page_path(@conn, :espace_producteur)}>
<a class="button" href={page_path(@conn, :espace_producteur, utm_source: "producer_infos_page")}>
<%= dgettext("page-producteurs", "Access your producer section") %>
</a>
<div class="pt-24">
Expand Down
8 changes: 8 additions & 0 deletions apps/transport/lib/transport_web/views/email_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,12 @@ defmodule TransportWeb.EmailView do
url = TransportWeb.Router.Helpers.resource_url(TransportWeb.Endpoint, :details, id)
link(title, to: url)
end

def espace_producteur_url(view_name) do
TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur,
utm_source: "transactional_email",
utm_medium: "email",
utm_campaign: to_string(view_name)
)
end
end
4 changes: 4 additions & 0 deletions apps/transport/priv/gettext/default.pot
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,7 @@ msgstr ""
#, elixir-autogen, elixir-format
msgid "Compare two GTFS files"
msgstr ""

#, elixir-autogen, elixir-format
msgid "Producer space"
msgstr ""
4 changes: 4 additions & 0 deletions apps/transport/priv/gettext/en/LC_MESSAGES/default.po
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,7 @@ msgstr ""
#, elixir-autogen, elixir-format
msgid "Compare two GTFS files"
msgstr ""

#, elixir-autogen, elixir-format
msgid "Producer space"
msgstr ""
4 changes: 4 additions & 0 deletions apps/transport/priv/gettext/fr/LC_MESSAGES/default.po
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,7 @@ msgstr "Loi climat et résilience"
#, elixir-autogen, elixir-format
msgid "Compare two GTFS files"
msgstr "Comparer deux GTFS"

#, elixir-autogen, elixir-format
msgid "Producer space"
msgstr "Espace producteur"
9 changes: 2 additions & 7 deletions apps/transport/test/support/conn_case.ex
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ defmodule TransportWeb.ConnCase do
{:ok, conn: ConnTest.build_conn()}
end

def setup_admin_in_session(conn) do
conn
|> init_test_session(%{
current_user: %{
"organizations" => [%{"slug" => "equipe-transport-data-gouv-fr"}]
}
})
def setup_admin_in_session(%Plug.Conn{} = conn) do
init_test_session(conn, %{current_user: %{"is_admin" => true}})
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ defmodule Transport.Test.Transport.Jobs.DatasetNowOnNAPNotificationJobTest do
~s(Votre jeu de données <a href="http://127.0.0.1:5100/datasets/#{dataset.slug}">Mon JDD</a> a bien été référencé)

assert html_content =~
~s(Rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur">Espace Producteur</a)
~s(Rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur?utm_source=transactional_email&amp;utm_medium=email&amp;utm_campaign=dataset_now_on_nap">Espace Producteur</a)

:ok
end)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ defmodule Transport.Test.Transport.Jobs.PeriodicReminderProducersNotificationJob
~s(Il est possible de vous inscrire à des notifications concernant le jeu de données que vous gérez sur transport.data.gouv.fr, <a href="http://127.0.0.1:5100/datasets/#{dataset.slug}">#{dataset.custom_title}</a>)

assert html =~
~s(Pour vous inscrire, rien de plus simple : rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur">Espace Producteur</a>)
~s(Pour vous inscrire, rien de plus simple : rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur?utm_source=transactional_email&amp;utm_medium=email&amp;utm_campaign=periodic_reminder_producer_without_subscriptions">Espace Producteur</a>)
end)

assert :ok == perform_job(PeriodicReminderProducersNotificationJob, %{"contact_id" => contact.id})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ defmodule Transport.Test.Transport.Jobs.ResourceUnavailableNotificationJobTest d
assert html_part =~ "Il semble que vous ayez supprimé puis créé une nouvelle ressource"

assert html_part =~
~s(rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur">Espace Producteur</a> à partir duquel vous pourrez procéder à ces mises à jour)
~s(rendez-vous sur votre <a href="http://127.0.0.1:5100/espace_producteur?utm_source=transactional_email&amp;utm_medium=email&amp;utm_campaign=resource_unavailable_producer">Espace Producteur</a> à partir duquel vous pourrez procéder à ces mises à jour)

:ok
end)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,18 @@ defmodule TransportWeb.BackofficeControllerTest do
assert get_flash(conn, :info) =~ "Vous devez être préalablement connecté"
end

test "Check that you belong to the right organization", %{conn: conn} do
test "Check that you are an admin", %{conn: conn} do
conn =
conn
|> init_test_session(%{current_user: %{"organizations" => [%{"slug" => "pouet pouet"}]}})
|> init_test_session(%{current_user: %{"is_admin" => false}})
|> get(backoffice_page_path(conn, :index))

assert redirected_to(conn, 302) =~ "/login/explanation"
assert get_flash(conn, :error) =~ "You need to be a member of the transport.data.gouv.fr team."
end

test "Show 'add new dataset' form", %{conn: conn} do
conn =
conn
|> init_test_session(%{
current_user: %{"organizations" => [%{"slug" => "blurp"}, %{"slug" => "equipe-transport-data-gouv-fr"}]}
})
|> get(backoffice_page_path(conn, :index))

conn = conn |> setup_admin_in_session() |> get(backoffice_page_path(conn, :index))
assert html_response(conn, 200) =~ "Ajouter un jeu de données"
end

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
defmodule TransportWeb.Backoffice.BrokenUrlsControllerTest do
use TransportWeb.ConnCase, async: true
import Plug.Test
import DB.Factory
alias TransportWeb.Backoffice.BrokenUrlsController

Expand All @@ -24,12 +23,7 @@ defmodule TransportWeb.Backoffice.BrokenUrlsControllerTest do
dataset_history_2 = insert(:dataset_history, dataset_id: dataset.id)
insert(:dataset_history_resources, dataset_history_id: dataset_history_2.id, payload: %{"download_url" => "url2"})

conn =
conn
|> init_test_session(%{
current_user: %{"organizations" => [%{"slug" => "equipe-transport-data-gouv-fr"}]}
})
|> get(backoffice_broken_urls_path(conn, :index))
conn = conn |> setup_admin_in_session() |> get(backoffice_broken_urls_path(conn, :index))

res = conn |> html_response(200)
assert res =~ "Détection de changements d'URLs stables"
Expand Down Expand Up @@ -85,12 +79,7 @@ defmodule TransportWeb.Backoffice.BrokenUrlsControllerTest do
dataset_history_2 = insert(:dataset_history, dataset_id: dataset.id)
insert(:dataset_history_resources, dataset_history_id: dataset_history_2.id, payload: %{"download_url" => "url1"})

conn =
conn
|> init_test_session(%{
current_user: %{"organizations" => [%{"slug" => "equipe-transport-data-gouv-fr"}]}
})
|> get(backoffice_broken_urls_path(conn, :index))
conn = conn |> setup_admin_in_session() |> get(backoffice_broken_urls_path(conn, :index))

# check we get a 200 if list is empty
res = conn |> html_response(200)
Expand Down
Loading