Skip to content

Conversation

NidhiDixit09
Copy link
Collaborator

Reference

EMT - 2337 : Allow for developers to pass us their Meta anon_id
https://branch.atlassian.net/browse/EMT-2337

Type Of Change

  • New feature (non-breaking change which adds functionality)

cc @BranchMetrics/saas-sdk-devs for visibility.

Copy link
Contributor

matter-code-review bot commented Aug 26, 2025

Code Quality bug detected new feature

Summary By MatterAI MatterAI logo

🔄 What Changed

This Pull Request, titled "Emt-2337 setAnonID API", indicates an intention to allow developers to pass a Meta anon_id. However, the only code change provided in the diff is a minor syntax correction in the .github/workflows/verify.yml file, changing steps:16 to steps:. The core implementation for the setAnonID API is not present in the provided changes. ⚠️

🔍 Impact of the Change

The direct impact of the provided code change is a fix for a malformed YAML syntax in the CI workflow, which ensures the verify job can execute correctly. The intended setAnonID API, if implemented, would allow for enhanced tracking capabilities by integrating Meta anon_id into the system. However, without the actual API implementation, this impact remains theoretical based on the PR description. 🚧

📁 Total Files Changed

  • .github/workflows/verify.yml: Corrected a YAML syntax error from steps:16 to steps:.

🧪 Test Added

No new functional tests were added in the provided code changes. The change is a CI configuration fix. ❌

🔒Security Vulnerabilities

The provided YAML syntax fix does not introduce any new security vulnerabilities. However, if the setAnonID API were to be implemented, it would require thorough input validation and security checks to prevent potential data manipulation or injection risks. 🛡️

Tip

Quality Recommendations

  1. Implement the setAnonID API as described in the PR title and body.

  2. Ensure the code changes in the PR accurately reflect its stated purpose and title.

  3. Add comprehensive unit and integration tests for the setAnonID API once implemented.

  4. Implement robust input validation for the anon_id parameter in the new API to prevent security vulnerabilities.

Tanka Poem ♫

YAML's small error,
Fixed now, CI runs with grace.
API, where art thou?
Anon ID, a promise waits,
Code's true form, yet to unfold. 🐛

@gdeluna-branch
Copy link
Contributor

/matter review

@BranchMetrics BranchMetrics deleted a comment from matter-code-review bot Aug 27, 2025
@BranchMetrics BranchMetrics deleted a comment from matter-code-review bot Aug 27, 2025
Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

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

Good implementation of setAnonID API with comprehensive test coverage. Found one potential issue with type casting that could cause runtime crashes.

Comment on lines +309 to +311
// Try to set with a non-string object (NSNumber)
NSNumber *nonStringValue = @123;
[Branch setAnonID:(NSString *)nonStringValue];
Copy link
Contributor

Choose a reason for hiding this comment

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

🐛 Bug Fix

Issue: Unsafe type casting of NSNumber to NSString will cause runtime crash when the method tries to validate the parameter type
Fix: Remove the explicit cast to let the method handle type validation naturally
Impact: Prevents potential crash and allows proper testing of type validation logic

Suggested change
// Try to set with a non-string object (NSNumber)
NSNumber *nonStringValue = @123;
[Branch setAnonID:(NSString *)nonStringValue];
// Try to set with a non-string object (NSNumber)
NSNumber *nonStringValue = @123;
[Branch setAnonID:nonStringValue];

@gdeluna-branch
Copy link
Contributor

I do have one question, if the anon id is set once, it will stay set. Should we have a clear method?

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

Copy link
Contributor

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@NidhiDixit09 NidhiDixit09 merged commit d7f6b62 into master Aug 29, 2025
9 of 15 checks passed
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