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 all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,10 @@ defmodule TransportWeb.PageController do
{conn, []}
end

conn |> assign(:datasets, datasets) |> render("espace_producteur.html")
conn
|> assign(:datasets, datasets)
|> TransportWeb.Session.set_is_producer(datasets)
|> render("espace_producteur.html")
end

AntoineAugusti marked this conversation as resolved.
Show resolved Hide resolved
defp aoms_with_dataset do
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 @@ -127,24 +127,17 @@ 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))
|> TransportWeb.Session.set_is_producer(user_params)
|> TransportWeb.Session.set_is_admin(user_params)
end

thbar marked this conversation as resolved.
Show resolved Hide resolved
@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]}
"""
def user_params_for_session(%{} = params) do
Map.put(
params,
"organizations",
Enum.filter(
params["organizations"],
&match?(%{"slug" => "equipe-transport-data-gouv-fr"}, &1)
)
)
defp user_params_for_session(%{} = params) do
# 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(params, "organizations")
end

defp get_redirect_path(%Plug.Conn{} = conn) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ defmodule TransportWeb.Backoffice.JobsLive do
# https://hexdocs.pm/phoenix_live_view/security-model.html#disconnecting-all-instances-of-a-given-live-user
#
def ensure_admin_auth_or_redirect(socket, current_user, func) do
if current_user && TransportWeb.Router.is_transport_data_gouv_member?(current_user) do
socket = assign(socket, current_user: current_user)

if TransportWeb.Session.is_admin?(socket) do
# We track down the current admin so that it can be used by next actions
socket = assign(socket, current_admin_user: current_user)
# Then call the remaining code, which is expected to return the socket
func.(socket)
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ defmodule TransportWeb.Backoffice.ProxyConfigLive do
"""
use Phoenix.LiveView
alias Transport.Telemetry
import TransportWeb.Backoffice.JobsLive, only: [ensure_admin_auth_or_redirect: 3]
import TransportWeb.Router.Helpers

# The number of past days we want to report on (as a positive integer).
Expand All @@ -20,25 +21,6 @@ defmodule TransportWeb.Backoffice.ProxyConfigLive do
end)}
end

#
# If one calls "redirect" and does not leave immediately, the remaining code will
# be executed, opening security issues. This method goal is to minimize this risk.
# See https://hexdocs.pm/phoenix_live_view/security-model.html for overall docs.
#
# Also, disconnect will have to be handled:
# https://hexdocs.pm/phoenix_live_view/security-model.html#disconnecting-all-instances-of-a-given-live-user
#
defp ensure_admin_auth_or_redirect(socket, current_user, func) do
if current_user && TransportWeb.Router.is_transport_data_gouv_member?(current_user) do
# We track down the current admin so that it can be used by next actions
socket = assign(socket, current_admin_user: current_user)
# Then call the remaining code, which is expected to return the socket
func.(socket)
else
redirect(socket, to: "/login")
end
end

defp schedule_next_update_data do
Process.send_after(self(), :update_data, 1000)
end
Expand All @@ -57,8 +39,7 @@ defmodule TransportWeb.Backoffice.ProxyConfigLive do
end

def handle_event("refresh_proxy_config", _value, socket) do
if socket.assigns.current_admin_user, do: config_module().clear_config_cache!()

config_module().clear_config_cache!()
{:noreply, socket}
end

Expand Down
10 changes: 2 additions & 8 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 @@ -352,13 +353,6 @@ defmodule TransportWeb.Router do
end
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

# 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
export_key_value = Map.get(params, "export_key", "")
Expand All @@ -375,7 +369,7 @@ defmodule TransportWeb.Router do
end

defp transport_data_gouv_member(%Plug.Conn{} = conn, _) do
if is_transport_data_gouv_member?(conn.assigns[:current_user]) do
if TransportWeb.Session.is_admin?(conn) do
conn
else
conn
Expand Down
67 changes: 67 additions & 0 deletions apps/transport/lib/transport_web/session.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
defmodule TransportWeb.Session do
@moduledoc """
Web session getters and setters.
"""
import Ecto.Query
import Plug.Conn

@is_admin_key_name "is_admin"
@is_producer_key_name "is_producer"

@doc """
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 and refreshed when you visit your "Espace producteur".
"""
@spec set_is_producer(Plug.Conn.t(), map() | [DB.Dataset.t()]) :: Plug.Conn.t()
def set_is_producer(%Plug.Conn{} = conn, %{"organizations" => _} = params) do
set_session_attribute_attribute(conn, @is_producer_key_name, is_producer?(params))
end

def set_is_producer(%Plug.Conn{} = conn, [%DB.Dataset{}] = _datasets_for_user) do
set_session_attribute_attribute(conn, @is_producer_key_name, true)
end

def set_is_producer(%Plug.Conn{} = conn, [] = _datasets_for_user) do
set_session_attribute_attribute(conn, @is_producer_key_name, false)
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.
"""
def set_is_admin(%Plug.Conn{} = conn, %{"organizations" => _} = params) do
set_session_attribute_attribute(conn, @is_admin_key_name, is_admin?(params))
end

def is_admin?(%{"organizations" => orgs}) do
Enum.any?(orgs, &(&1["slug"] == "equipe-transport-data-gouv-fr"))
end

def is_admin?(%Plug.Conn{} = conn) do
conn |> current_user() |> Map.get(@is_admin_key_name, false)
end

def is_admin?(%Phoenix.LiveView.Socket{assigns: %{current_user: current_user}}) do
Map.get(current_user, @is_admin_key_name, false)
end

def is_producer?(%Plug.Conn{} = conn) do
conn |> current_user() |> Map.get(@is_producer_key_name, false)
end

def is_producer?(%{"organizations" => orgs}) do
org_ids = Enum.map(orgs, & &1["id"])
DB.Dataset.base_query() |> where([dataset: d], d.organization_id in ^org_ids) |> DB.Repo.exists?()
end

@spec set_session_attribute_attribute(Plug.Conn.t(), binary(), boolean()) :: Plug.Conn.t()
defp set_session_attribute_attribute(%Plug.Conn{} = conn, key, value) do
current_user = current_user(conn)
conn |> put_session(:current_user, Map.put(current_user, key, value))
end

defp current_user(%Plug.Conn{} = conn), do: get_session(conn, :current_user, %{})
end
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={TransportWeb.Session.is_admin?(@conn)} 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 TransportWeb.Session.is_admin?(@conn) 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 TransportWeb.Session.is_admin?(@conn) 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 <%= link_for_espace_producteur(: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 <%= link_for_espace_producteur(: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 <%= link_for_espace_producteur(: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 <%= link_for_espace_producteur(: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 TransportWeb.Session.is_admin?(@conn) do %>
<%= link("Administration", to: "/backoffice") %>
<% end %>
<%= if TransportWeb.Session.is_producer?(@conn) 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
11 changes: 11 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,15 @@ defmodule TransportWeb.EmailView do
url = TransportWeb.Router.Helpers.resource_url(TransportWeb.Endpoint, :details, id)
link(title, to: url)
end

def link_for_espace_producteur(view_name) do
url =
TransportWeb.Router.Helpers.page_url(TransportWeb.Endpoint, :espace_producteur,
utm_source: "transactional_email",
utm_medium: "email",
utm_campaign: to_string(view_name)
)

link("Espace Producteur", to: url)
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
Loading