-
Notifications
You must be signed in to change notification settings - Fork 180
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
Analytics for quick edit modals, relative to legacy edit modal #4696
Analytics for quick edit modals, relative to legacy edit modal #4696
Conversation
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.
Hi @bjester - this looks so great, thank you for working on it. Overall this seems to be working as expected with the manual QA that I've done. I noticed a couple of non-blocking potential improvements to make things more clear to analyze that we can decide whether or not we want to update now, or if we want to do this in a patch release.
The first is that in the legacy edit modal, I think due to the autofocus in the title, there is a tracked click event every time the modal opens, which Sairina introduced a couple of years ago. I don't think this necessarily has to change, and it has a different event name, but it might just be something worth making note of if/when we do analytics so that we are not counting each of those as an intentional title edit.
My other question is whether or not the "specific" changed details would be useful to have in the legacy modal more comprehensively, or if it's better on the analytics side to draw on the existing data and merge the two sources of info for analysis. (And, this really isn't a code implementation question, it's an analytics question that I just don't know the answer to).
For example, with the quick edit modals, we can see this in the console:
which to me is very clear. But also, the modals are narrowly scoped in terms of what's happening :)
The legacy edit modal has events that track both of this, but they're split up, a bit, with both the new additions you've made, plus the existing analytics.
Do you think on the google analytics side we'll be able to do comparisons like this, or is having more "detail" in the updates that you added useful?
Definitely not a blocker, just a curiosity as it's been a really long time since I've done anything GA and that was mostly in terms of email and social media link click through stuff.
@marcellamaki I thought I had attached the notion document to the PR description, but it seems I didn't-- here it is. There's only one signal related to tracking the legacy types of editing, and I didn't attempt to track it because for legacy edits we'd need to report on it at the user level, based off how the tracking works. So without a split test, I don't know that we can get usable data for direct comparisons between the two for different types fields / edits the user makes in each. For the quick edit modals, it makes sense to differentiate between individual edit events because they're different modals, those signals are requested, and it doesn't hurt to track that. With the legacy edit modal, they can really change anything, so if our primary signal is in time spent curating, the legacy modal could very well out perform the quick edits because a user can change multiple things at once. |
Ah this Notion doc is great!! Thank you! Answers my questions (and so many more I didn't know I had). Very cool. Okay, well I don't have any blocking concerns on this. Not sure if @AlexVelezLl or @rtibbles want to weigh in on anything? If not, I'll go ahead and approve. |
@marcellamaki we can also leverage the prior existing events for reporting on this feature. Most of this work just makes it easier to set things up in GTM for tracking, which Jessica, Tomiwa and I found out in our last meeting when we tried to attach click events to the modals 😅 |
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.
Thank you @blaine! I just saw that there are differences in what we consider as changed across the Quick Edit Modals. For example in modals like the boolean maps (categories, learning activities, levels, resources needed), the completion and the source we consider changed = true
if the user interacted with the form in the modal, as we set watchers that set the changed
value if we ever changed anything, but it doesnt take into acount if we reverted the change in a later state. Meanwhile in modals like the audience, the language, the Title and Description we consider changed = true
if the current state on save is different from the initial state of the node before persisting the changes.
For example if we do something like this in the edit audience modal:
Initial state: Author = "Author 1"
Change 1: Author = "Author 2"
Change 2: Author = "Author 1"
We will mark this as changed: true.
But if we do something like this in the edit title and description modal:
Initial state: Title = "Node 1"
Change 1: Title = "Node 2"
Change 2: Title = "Node 1"
We will mark this as changed: false.
Compartir.pantalla.-.2024-09-03.14_10_56.mp4
Im not sure which should be the correct behaviour, If changed
means that we interacted with the form, or if it means that the state was changed comparing to the initial state. But I think this should be consistent in all quick edit modals.
Apart from that, everything else looks good to me :)
Oo that's a good edge case/clarification, @AlexVelezLl! thanks for noticing that. |
@AlexVelezLl you're right, I did this quickly and mixed the two. we only really care that an interaction occurred, so just the ones comparing with initial state need to be fixed, and those are probably less likely to return to their original state if the user edited them |
Oh, If we are ok with that behaviour its ok on my side 😅. Would be worth opening a follow up or informing somewhere in the code that this difference in tracking the changes is on purpose |
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.
Lgtm! 🤗
Summary
Description of the change(s) you made
Manual verification steps performed
Reviewer guidance
How can a reviewer test these changes?
Are there any risky areas that deserve extra testing?
References
Events and data should support the desired tracking defined in Confirm the analytics data in the console-- that it's accurate and representative of the changes
Notion doc for reference: https://www.notion.so/learningequality/Use-Studio-tag-manager-to-track-the-bulk-editing-feature-update-a225bb594ddf4571aa02d4ad349a92be?pvs=4
Comments
Contributor's Checklist
PR process:
CHANGELOG
label been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocs
label has been added if this introduces a change that needs to be updated in the user docs?requirements.txt
files also included in this PRStudio-specifc:
notranslate
class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages
,components
, andlayouts
directories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarn
andpip
)