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

Fixing removing white space identifiers #201

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

ajaysubra
Copy link
Contributor

Description

When identifiers were just a bunch of whitespace characters, we were still making a network call and getting 400'ed. This fixes that by trimming white spaces and newline before checking if the identifiers is empty.

Check List

  • Are you changing anything with the public API?
  • Have you tested this change on real device?
  • Are your changes backwards compatible with previous SDK Versions?
  • Have you added unit test coverage for your changes?
  • Have you verified that your changes are compatible with all the operating system version this SDK currently supports?

Manual Test Plan

  1. Add an empty identifier to the profile (white spaces would work too) - email, phone number or external id
  2. Tap on create profile
  3. Notice that a request will be made (since we don't have dedup logic for profile requests) but the request will not contain the empty identifier that was added.

@ajaysubra ajaysubra requested a review from a team as a code owner September 17, 2024 04:04
Copy link
Contributor

@evan-masseau evan-masseau left a comment

Choose a reason for hiding this comment

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

Minor, but I log a warning message whenever we make a modification to a value, such as whitespace removal

@ajaysubra
Copy link
Contributor Author

Minor, but I log a warning message whenever we make a modification to a value, such as whitespace removal

We already do before we get here in the state management file.

@ajaysubra
Copy link
Contributor Author

Minor, but I log a warning message whenever we make a modification to a value, such as whitespace removal

We already do before we get here in the state management file.

However that says we ignore the requested change and not trimming white spaces -

private func logDevWarning(for identifier: String) {
    environment.emitDeveloperWarning("""
    \(identifier) is either empty or same as what is already set earlier.
    The SDK will ignore this change, please use resetProfile for
    resetting profile identifiers
    """)
}

@ajaysubra
Copy link
Contributor Author

Minor, but I log a warning message whenever we make a modification to a value, such as whitespace removal

We already do before we get here in the state management file.

However that says we ignore the requested change and not trimming white spaces -

private func logDevWarning(for identifier: String) {
    environment.emitDeveloperWarning("""
    \(identifier) is either empty or same as what is already set earlier.
    The SDK will ignore this change, please use resetProfile for
    resetting profile identifiers
    """)
}

Kinda true that we do ignore the identifier that is empty so I think what we have is sufficient without getting into too much detail. LMK what you think.

@ajaysubra ajaysubra requested review from ab1470 and ndurell September 17, 2024 16:30
@ajaysubra ajaysubra merged commit 3e62701 into master Sep 17, 2024
9 checks passed
@ajaysubra ajaysubra deleted the as/fixing-empty-identifiers branch September 17, 2024 17:25
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.

4 participants