-
Notifications
You must be signed in to change notification settings - Fork 49
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
KB-45700: Update Telemeter Tracking to Utilize Promises #954
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.
To make sure the applied changes really have effect, we´d have to await
all the methods we appended the async
keyword to... In other words, for example, now that we made insertFormula
an async
method, we need to check all places where that method is used and await its execution... This could bring us to an infinity spiral of async/await methods, let's check it at least!
The code is thought to run in serial mode, so it may not be an issue to add the async/await where needed. However, take into consideration that there are cyclic methods. |
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.
Left a comment regarding some refactor code that was interesting to be included in this PR.
4c26f04
to
a3a43ff
Compare
Sorry for the length of this PR. Fixed a bunch of typos and added a more considerate way to track the closing telemetry. |
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.
Even though there are interesting changes improving the code, they are removing important telemetry calls and mixing some of them. In the comments, i specified which ones.
2992ad8
to
d828e51
Compare
fix: change debug mode to false refactor: change the way the promise is handled revert: Only the close telemeter track is async fix: change quote types
…eds to and fixed typos fix: miss to restore debug mode to false fix: Generic telemetry work and correct some misc errors with methods fix: Add the trigger capability to a global attribute docs: Update docs of some functions to explain what it does fix: remove unwanted files fix: restore unwanted files fix: debug mode to false and changed telemeter object
Changes
feat: update telemeter tracking to utilize promises. #KB-45700
Description
This PR introduces a significant enhancement to our telemeter tracking functionality by transitioning all tracking actions to utilize promises. The motivation behind this change stems from a recent integration issue encountered with OnlyOffice.
Steps to Reproduce:
wiris/telemeter
repo.cargo run
and ensure it's operational.packages/devkit/src/Integrationmodel
on this branch.debug: false
todebug: true
.yarn && nx build generic && nx start html-generic
to build the generic editor.Commands to test generic telemetry
WirisPlugin.GenericIntegration.telemeter.wrsOpenedEditorModal('button','chemistry')
WirisPlugin.GenericIntegration.telemeter.wrsClosedEditorModal('mtct_close','chemistry')
WirisPlugin.GenericIntegration.telemeter.wrsInsertedFormula('«math xmlns=¨http://www.w3.org/1998/Math/MathML¨»«msqrt»«mi»x«/mi»«/msqrt»«/math»','«math xmlns=¨http://www.w3.org/1998/Math/MathML¨»«msqrt»«mi»x«/mi»«/msqrt»«/math»',Date.now(),'chemistry')
Link to Task: #45700