Skip to content

Improved the analytics and log events.#350

Merged
amitojsingh merged 7 commits into
devfrom
add-analytics
Dec 22, 2025
Merged

Improved the analytics and log events.#350
amitojsingh merged 7 commits into
devfrom
add-analytics

Conversation

@amitojsingh

Copy link
Copy Markdown
Collaborator

Earlier, multiple events were being recorded for the same screen, so this update will resolve that issue.
Another issue was that Sundar Gutka screens were being recorded directly from React Native instead of the proper navigation flow, and I’ve fixed that as well in this PR.

Comment thread src/navigation/index.jsx
onReady={() => {
routeNameRef.current = navigationRef.current.getCurrentRoute().name;
}}
onStateChange={async (state) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Minor thing, this can be modularized, create a separate function for this logic, and then call it here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/navigation/index.jsx Outdated
routeNameRef.current = navigationRef.current.getCurrentRoute().name;
}}
onStateChange={async (state) => {
handlingStateChange(state);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The handlingStateChange function is async but called without await in the onStateChange handler. Any specific reason for this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I was uncertain about using await here because I want the navigation to continue immediately without being blocked.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/common/middleware/crashlytics.js Outdated
const summarizeState = (state) => {
const summary = {};
let count = 0;
Object.entries(state || {}).some(([key, value]) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It works, but is slightly weird use case of some(). What if we keep it simple with a for loop with a break statement if count is greater than/equal to MAX_STATE_KEYS

for (const [key, value] of Object.entries(state || {})) {
  if (count >= MAX_STATE_KEYS) break;
  if (value === undefined) continue;

  if (
    typeof value === "string" ||
    typeof value === "number" ||
    typeof value === "boolean"
  ) {
    const crashlyticsKey = key.replace(/([A-Z])/g, "-$1").toLowerCase();
    summary[crashlyticsKey] = safeStringify(value);
    count += 1;
  }
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

According to the Airbnb JavaScript style guide, for-of and for-in loops should be avoided in favor of array iteration methods like .map(), .forEach(), and .reduce().
airbnb/javascript#1271

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think that depends on the use case. Here's a detailed explanation by chatgpt

https://chatgpt.com/share/693a69a9-40d4-8010-833b-6808e4fc4d6e

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@amitojsingh what did we change here? Did we change anything?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

replaced the code.


useEffect(() => {
// Track screen view when error fallback is shown
trackScreenView("ErrorFallback", null, "Error Boundary Fallback Screen");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why aren't we using the constant variable constant.FALLBACK here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

…king

- Updated FallbackComponent to use a constant for screen tracking.
- Enhanced Navigation component to handle state changes more robustly, including error logging for performance trace failures and improved screen view tracking.
- Replaced Object.entries with a for...of loop for better readability and performance.
- Updated logic to skip undefined values using continue statement, improving clarity of the code.
* Enhance audio tracking and language validation in HomeScreen and AudioPlayer components

- Added language validation logic in HomeScreen to ensure valid language selection.
- Integrated new analytics tracking for audio events, including artist listening duration and audio requests.
- Updated AudioControlBar to utilize the new useArtistListeningDuration hook for tracking listening time.
- Refactored AudioTrackDialog to include analytics tracking for audio link requests.
- Improved import structure in various components for better organization and clarity.

* fixed
- Integrated analytics tracking for audio link requests by adding trackAudioEvent.
- Updated buttonPress logic to handle errors gracefully when opening the audio request URL.
- Improved user experience by ensuring a fallback mechanism for URL opening failures.
@amitojsingh amitojsingh merged commit b85fc73 into dev Dec 22, 2025
3 checks passed
@amitojsingh amitojsingh deleted the add-analytics branch December 22, 2025 21:26
smanjot27 pushed a commit to manjotkaur27/sundar-gutka-react that referenced this pull request Mar 28, 2026
* Improved the analytics and log events.

* Refactor FallbackComponent and Navigation for improved analytics tracking

- Updated FallbackComponent to use a constant for screen tracking.
- Enhanced Navigation component to handle state changes more robustly, including error logging for performance trace failures and improved screen view tracking.

* Refactor summarizeState function in crashlytics middleware

- Replaced Object.entries with a for...of loop for better readability and performance.
- Updated logic to skip undefined values using continue statement, improving clarity of the code.

* Added analytics for Sundar Gutka Audio (KhalisFoundation#351)

* Enhance audio tracking and language validation in HomeScreen and AudioPlayer components

- Added language validation logic in HomeScreen to ensure valid language selection.
- Integrated new analytics tracking for audio events, including artist listening duration and audio requests.
- Updated AudioControlBar to utilize the new useArtistListeningDuration hook for tracking listening time.
- Refactored AudioTrackDialog to include analytics tracking for audio link requests.
- Improved import structure in various components for better organization and clarity.

* fixed

* fix: test

* Enhance audio link request tracking in AudioPlayer component

- Integrated analytics tracking for audio link requests by adding trackAudioEvent.
- Updated buttonPress logic to handle errors gracefully when opening the audio request URL.
- Improved user experience by ensuring a fallback mechanism for URL opening failures.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants