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

Overlay fix and GA events for error modal #46

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@internetarchive/ia-book-actions",
"version": "0.0.23",
"version": "0.0.23-1",
"description": "Bookreader actions for borrowable book",
"author": "Internet Archive",
"license": "AGPL-3.0-only",
Expand Down
2 changes: 1 addition & 1 deletion src/assets/styles/collapsible-action-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export default css`
}
.open {
display: block;
z-index: 2;
z-index: 4;
}
.visible {
display: inline-block;
Expand Down
19 changes: 15 additions & 4 deletions src/core/services/loan-token-poller.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ActionsHandlerService from './actions-handler/actions-handler-service.js';
import * as Cookies from './doc-cookies.js';
import { analyticsCategories } from '../config/analytics-event-and-category.js';

/**
* This class is used to create loan token for borrowed books
Expand All @@ -13,6 +14,7 @@ export class LoanTokenPoller {
this.successCallback = successCallback; // callback function to be called after loan token is created
this.errorCallback = errorCallback; // callback function to be called after loan token is created
this.pollerDelay = pollerDelay; // value in ms (1000 ms = 1 sec)
this.analyticsCategories = analyticsCategories;

this.loanTokenInterval = undefined;
this.enableBookAccess();
Expand Down Expand Up @@ -69,13 +71,14 @@ export class LoanTokenPoller {
}
}

sendEvent(eventCategory, eventAction) {
sendEvent(eventCategory, eventAction, extraParams = {}) {
// eslint-disable-next-line no-console
console?.log('Book action: ', { eventCategory, eventAction });
console?.log('Book action: ', { eventCategory, eventAction, extraParams });
window?.archive_analytics?.send_event_no_sampling(
eventCategory,
eventAction,
`identifier=${this.identifier}`
`identifier=${this.identifier}`,
extraParams
);
}

Expand All @@ -85,7 +88,15 @@ export class LoanTokenPoller {
identifier: this.identifier,
action,
error: data => {
window?.Sentry?.captureMessage('handleLoanTokenPoller error');
window?.Sentry?.captureMessage(`
handleLoanTokenPoller error, identifier:${this.identifier}, borrowType:${this.borrowType}, interval:${this.loanTokenInterval}
`);
this.sendEvent(this.analyticsCategories.browse, 'LoanTokenError', {
identifier: this.identifier,
isInitial: isInitial,
borrowType: this.borrowType,
interval: this.loanTokenInterval,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: this.loanTokenInterval - seems really heavy to send to GA. can u send this as a boolean or a string like an errorType?

the one we want to catch is if a book has been auto-returned and the interval is polling. is that borrowType?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setInterval does not return a promise, but an integer number, uniquely identifying the interval timer that was created. so not sure how this heavy to send GA. I just wanted to know if this really have a identification number even after get cleared.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nsharma123 is this PR still good?

});
this.disconnectedInterval(); // stop token fetch api
this.errorCallback({ detail: { action, data } });
},
Expand Down