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

refactor: move datadog rum methods to service (DEV-190) #572

Merged
merged 5 commits into from Nov 5, 2021

Conversation

mdelez
Copy link
Collaborator

@mdelez mdelez commented Nov 2, 2021

resolves DEV-190

@mdelez mdelez added the refactor Refactor or redesign label Nov 2, 2021
@mdelez mdelez self-assigned this Nov 2, 2021
@mdelez mdelez requested a review from subotic Nov 2, 2021
) {

// set the page title
this._titleService.setTitle('DaSCH Service Platform');

// init datadog RUM
this._datadogRumService.initializeRum();
Copy link
Contributor

@subotic subotic Nov 5, 2021

Choose a reason for hiding this comment

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

Why is the datadogRumService initialized here and not at time of construction?

Copy link
Collaborator Author

@mdelez mdelez Nov 5, 2021

Choose a reason for hiding this comment

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

changed in a48ea3d

}
}

setActiveUser(identifier: any, identifierType: 'iri' | 'email' | 'username'): void {
Copy link
Contributor

@subotic subotic Nov 5, 2021

Choose a reason for hiding this comment

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

you need to check if datadogRum is initialized, and only then set the user. Otherwise you can probably ignore it.

Copy link
Collaborator Author

@mdelez mdelez Nov 5, 2021

Choose a reason for hiding this comment

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

changed in a48ea3d

}

removeActiveUser(): void {
datadogRum.removeUser();
Copy link
Contributor

@subotic subotic Nov 5, 2021

Choose a reason for hiding this comment

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

likewise, check if datadogRum is initialized.

Copy link
Collaborator Author

@mdelez mdelez Nov 5, 2021

Choose a reason for hiding this comment

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

changed in a48ea3d

@mdelez mdelez requested a review from subotic Nov 5, 2021
subotic
subotic approved these changes Nov 5, 2021
@kilchenmann kilchenmann merged commit 77191cb into main Nov 5, 2021
12 checks passed
@kilchenmann kilchenmann deleted the wip/dev-190-datadog-rum-init-refactor branch Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactor or redesign
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants