Conversation
dawei-nava
force-pushed
the
dwang/LG-11037-empty-event-logged
branch
from
87f6e76 to
36823e3
Compare
changelog: Internal, Logging enhancement, Missing step names for analytics events.
dawei-nava
requested review from
a team and
charleyf
and removed request for
a team
| expect(page).to have_link( | ||
| t('links.cancel'), | ||
| href: idv_cancel_path, | ||
| href: idv_cancel_path(step: :session, location: :warning), |
There was a problem hiding this comment.
Just curious: is there a reason we had to add a location here and to unavailable and not to the others?
There was a problem hiding this comment.
@night-jellyfish, not really, from what i see the location mostly means where the link is on the page, some easy to tell, some not.
There was a problem hiding this comment.
Interesting! I imagine this would be useful when trying to determine where someone cancelled from.
If so, could we add it to some of the others we're changing here, like app/views/sign_up/completions/show.html.erb, since we know that location?
If this is out of scope, I understand. I just thought that if we're adding it in some places, it might be helpful to add it to others as well.
There was a problem hiding this comment.
@night-jellyfish , i am removing the location param from here. First it's clear that cancel usually located at the botoom(footer). Second, it's more vague on definition side.
|
|
||
| <%= render PageFooterComponent.new do %> | ||
| <%= link_to t('links.cancel'), idv_cancel_path %> | ||
| <%= link_to t('links.cancel'), idv_cancel_path(step: :session, location: :warning) %> |
There was a problem hiding this comment.
@dawei-nava It seems odd to me to put "session" as the step. I didn't see session being used as the step elsewhere in the codebase. Could you please walk me through the decision to put session as the step? Thanks.
There was a problem hiding this comment.
@eileen-nava , this is a general session/auth landing page, like from partner agencies. It's more like a general function area about whether user has a valid session or not. Technically user can be redirected to here from any where?
What do you suggest to use?
There was a problem hiding this comment.
@dawei-nava Ahhhh, okay, that's helpful, thanks. I was confused because I expected the step to be a specific point in the user flow, like "verify info."
Would it be possible to put where the user is redirected from? If that isn't straightforward, I think "invalid_session" works.
Thanks for clarifying. 🙏🏻
4 participants
🎫 Ticket
LG-11037
🛠 Summary of changes
Add missing step name where analytics events logged.