-
Notifications
You must be signed in to change notification settings - Fork 297
runfix(cells): disable spammy tracking [WPB-21469] #19829
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
Changes from all commits
2c6d291
2286e09
d888223
3674ef4
7b665cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,12 +19,6 @@ | |||||
|
|
||||||
| import {useState, useCallback, useRef, SyntheticEvent} from 'react'; | ||||||
|
|
||||||
| import {amplify} from 'amplify'; | ||||||
|
|
||||||
| import {WebAppEvents} from '@wireapp/webapp-events'; | ||||||
|
|
||||||
| import {EventName} from 'Repositories/tracking/EventName'; | ||||||
|
|
||||||
| import {isVideoPlayable} from './isVideoPlayable/isVideoPlayable'; | ||||||
|
|
||||||
| interface UseVideoPlaybackProps { | ||||||
|
|
@@ -59,8 +53,9 @@ | |||||
| if (!playable) { | ||||||
| const status = 'unplayable'; | ||||||
|
|
||||||
| amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | ||||||
| amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.UNPLAYABLE_ERROR); | ||||||
| // Todo: This needs to be revisited | ||||||
|
Check warning on line 56 in src/script/components/MessagesList/Message/ContentMessage/asset/MultipartAssets/VideoAssetCard/VideoAssetPlayer/useVideoPlayback/useVideoPlayback.ts
|
||||||
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | ||||||
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.UNPLAYABLE_ERROR); | ||||||
| setIsError(true); | ||||||
| playabilityStatusRef.current = status; | ||||||
| return status; | ||||||
|
|
@@ -108,7 +103,8 @@ | |||||
|
|
||||||
| const handleError = useCallback((event: SyntheticEvent<HTMLVideoElement>) => { | ||||||
| setIsError(true); | ||||||
| amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | ||||||
| // Todo: This needs to be revisited | ||||||
|
Check warning on line 106 in src/script/components/MessagesList/Message/ContentMessage/asset/MultipartAssets/VideoAssetCard/VideoAssetPlayer/useVideoPlayback/useVideoPlayback.ts
|
||||||
|
||||||
| // Todo: This needs to be revisited | |
| // TODO: This needs to be revisited |
Copilot
AI
Dec 2, 2025
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.
Missing space after // in comment. For consistency with line 56 in this file, add a space: // amplify.publish
| //amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,12 +19,10 @@ | |||||||||
|
|
||||||||||
| import {useCallback, useEffect, useState} from 'react'; | ||||||||||
|
|
||||||||||
| import {amplify} from 'amplify'; | ||||||||||
| import cx from 'classnames'; | ||||||||||
| import {container} from 'tsyringe'; | ||||||||||
|
|
||||||||||
| import {TabIndex, Button, ButtonVariant, useTimeout} from '@wireapp/react-ui-kit'; | ||||||||||
| import {WebAppEvents} from '@wireapp/webapp-events'; | ||||||||||
|
|
||||||||||
| import {RestrictedVideo} from 'Components/asset/RestrictedVideo'; | ||||||||||
| import {AssetError} from 'Repositories/assets/AssetError'; | ||||||||||
|
|
@@ -33,7 +31,6 @@ | |||||||||
| import type {ContentMessage} from 'Repositories/entity/message/ContentMessage'; | ||||||||||
| import type {FileAsset as FileAssetType} from 'Repositories/entity/message/FileAsset'; | ||||||||||
| import {TeamState} from 'Repositories/team/TeamState'; | ||||||||||
| import {EventName} from 'Repositories/tracking/EventName'; | ||||||||||
| import {useKoSubscribableChildren} from 'Util/ComponentUtil'; | ||||||||||
| import {t} from 'Util/LocalizerUtil'; | ||||||||||
| import {formatSeconds} from 'Util/TimeUtil'; | ||||||||||
|
|
@@ -94,10 +91,11 @@ | |||||||||
| const video = document.createElement('video'); | ||||||||||
| const canPlay = video.canPlayType(mimeType) !== ''; | ||||||||||
|
|
||||||||||
| if (!canPlay) { | ||||||||||
| amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | ||||||||||
| amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.UNSUPPORTED_MIME_TYPE); | ||||||||||
| } | ||||||||||
| // ToDo: This needs to be revisited | ||||||||||
|
Check warning on line 94 in src/script/components/MessagesList/Message/ContentMessage/asset/VideoAsset/VideoAsset.tsx
|
||||||||||
|
||||||||||
| // ToDo: This needs to be revisited | |
| // Todo: This needs to be revisited |
Copilot
AI
Dec 2, 2025
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.
[Suggestion] Commented-out code should generally be removed rather than left in the codebase. If this tracking is truly temporary and will be revisited soon, consider:
- Creating a GitHub issue to track the follow-up work and reference it in the comment
- Removing the imports that are no longer used (already done ✓)
- Using a feature flag instead to disable tracking if you need to quickly re-enable it
If the tracking won't be re-enabled, remove the commented code entirely to improve maintainability.
| // if (!canPlay) { | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.UNSUPPORTED_MIME_TYPE); | |
| // } |
Check warning on line 137 in src/script/components/MessagesList/Message/ContentMessage/asset/VideoAsset/VideoAsset.tsx
SonarQubeCloud / SonarCloud Code Analysis
Complete the task associated to this "TODO" comment.
See more on https://sonarcloud.io/project/issues?id=wireapp_wire-webapp&issues=AZrgAt7pVOp-QAgK5IDK&open=AZrgAt7pVOp-QAgK5IDK&pullRequest=19829
Copilot
AI
Dec 2, 2025
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.
Inconsistent TODO format. The codebase uses "Todo" in the other file (useVideoPlayback.ts:56) but "ToDo" here. Please standardize to one format for consistency.
Copilot
AI
Dec 3, 2025
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.
Inconsistent comment format: "ToDo" should be "TODO" (standard task marker format).
Check warning on line 146 in src/script/components/MessagesList/Message/ContentMessage/asset/VideoAsset/VideoAsset.tsx
SonarQubeCloud / SonarCloud Code Analysis
Complete the task associated to this "TODO" comment.
See more on https://sonarcloud.io/project/issues?id=wireapp_wire-webapp&issues=AZrgAt7pVOp-QAgK5IDL&open=AZrgAt7pVOp-QAgK5IDL&pullRequest=19829
Copilot
AI
Dec 2, 2025
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.
Inconsistent TODO format. The codebase uses "Todo" in the other file (useVideoPlayback.ts:56) but "ToDo" here. Please standardize to one format for consistency.
Copilot
AI
Dec 3, 2025
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.
Inconsistent comment format: "ToDo" should be "TODO" (standard task marker format).
Copilot
AI
Dec 2, 2025
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.
Missing space after // in comment. For consistency with other comments in this file and line 107, add a space: // amplify.publish
| //amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_SUCCESS); | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_SUCCESS); |
Check warning on line 154 in src/script/components/MessagesList/Message/ContentMessage/asset/VideoAsset/VideoAsset.tsx
SonarQubeCloud / SonarCloud Code Analysis
Complete the task associated to this "TODO" comment.
See more on https://sonarcloud.io/project/issues?id=wireapp_wire-webapp&issues=AZrgAt7pVOp-QAgK5IDM&open=AZrgAt7pVOp-QAgK5IDM&pullRequest=19829
Copilot
AI
Dec 2, 2025
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.
Inconsistent TODO format. The codebase uses "Todo" in the other file (useVideoPlayback.ts:56) but "ToDo" here. Please standardize to one format for consistency.
Copilot
AI
Dec 3, 2025
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.
Inconsistent comment format: "ToDo" should be "TODO" (standard task marker format).
Copilot
AI
Dec 2, 2025
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.
Missing space after // in comment. For consistency with other comments in this file and line 107, add a space: // amplify.publish
| //amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); |
Check warning on line 220 in src/script/components/MessagesList/Message/ContentMessage/asset/VideoAsset/VideoAsset.tsx
SonarQubeCloud / SonarCloud Code Analysis
Complete the task associated to this "TODO" comment.
See more on https://sonarcloud.io/project/issues?id=wireapp_wire-webapp&issues=AZrgAt7pVOp-QAgK5IDN&open=AZrgAt7pVOp-QAgK5IDN&pullRequest=19829
Copilot
AI
Dec 2, 2025
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.
Inconsistent TODO format. The codebase uses "Todo" in the other file (useVideoPlayback.ts:56) but "ToDo" here. Please standardize to one format for consistency.
| // ToDo: This needs to be revisited | |
| // Todo: This needs to be revisited |
Copilot
AI
Dec 3, 2025
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.
Inconsistent comment format: "ToDo" should be "TODO" (standard task marker format).
| // ToDo: This needs to be revisited | |
| // TODO: This needs to be revisited |
Copilot
AI
Dec 2, 2025
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.
Missing space after // in comment. For consistency with other comments in this file and line 107, add a space: // amplify.publish
| //amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); | |
| // amplify.publish(WebAppEvents.ANALYTICS.EVENT, EventName.MESSAGES.VIDEO.PLAY_FAILED); |
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.
Inconsistent comment format: "Todo" should be "TODO" (standard task marker format). Also note this file uses "Todo" while VideoAsset.tsx uses "ToDo" - both should be standardized to "TODO".