-
Notifications
You must be signed in to change notification settings - Fork 91
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
tech(redirect): ETQ usager non connecté, je suis automatiquement redirigé de demarches-simplifiees vers le nouvel host #10042
tech(redirect): ETQ usager non connecté, je suis automatiquement redirigé de demarches-simplifiees vers le nouvel host #10042
Conversation
e84748e
to
920ae98
Compare
app/helpers/application_helper.rb
Outdated
def auto_switch_domain?(request, user_signed_in) | ||
maybe_switch_domain_enabled = request.params.key?(:switch_domain) || Flipper.enabled?(:switch_domain) | ||
|
||
maybe_switch_domain_enabled && !user_signed_in && app_host_legacy?(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pas user_signed_in?
? (avec le ?)
et je me rappelle plus pourquoi on veut le fairei que n'étant pas connecté, on en a du en parler mais j'ai zappé. En tout cas ok peut se limiter à ça au début
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
si tu peux rajouter un helper
def switch_domain_enabled?
request.params.key?(:switch_domain) || Flipper.enabled?(:switch_domain)
end
comme ça je reprend la même pour l'affichage de la bannière
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
user_signed_in?
, c'est un helper devise, pas dispo ds les specs des helpers. bref, d'ordre general, je prefere ne pas me reposer sur trop de dependance ds la methode, mais les donner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pour la question du prk en user_signed_in?
, je crois qu'on voulait faire de l'opt-in, sinon on aurait redirigé automatiquement via le token. en tous cas, c'est ds le plan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je crois qu'on parlait de l'optin si on ne faisait pas de détection auto. De toute façon je suis plutôt de voir ce que donne en vrai la détection, et si c'est OK assez rapidement migrer tout le monde quand elle est OK
fetch(hintUrl) | ||
.then(function(){ | ||
window.location = window.location.href.replace("#{ApplicationHelper::APP_HOST_LEGACY}", "#{ApplicationHelper::APP_HOST}") | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
il se passe quoi quand ça fail ?
A terme on pourrait imaginer ping sentry en catchant et re-throwant une erreur bien spécifique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pr le moment rien, car on souhaite ne pas rediriger :D mais oui ça fait partie du plan https://pad.numerique.gouv.fr/T4UcePBKR6KGzbjsWQJylw#
920ae98
to
a135947
Compare
c0d1205
No description provided.