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

Add basic validations to identifier setters #157

Merged
merged 4 commits into from
Apr 15, 2024

Conversation

evan-masseau
Copy link
Contributor

@evan-masseau evan-masseau commented Apr 11, 2024

Description

Adds a few very basic validations to the setter functions for identifiers. This will cut down on some avoidable API errors and duplicative API calls.

Check List

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

Changelog / Code Overview

  • Strip whitespace from identifiers
  • Reject an empty string identifier and log a warning
  • When an identifier is set using fluent setter functions, if it is the same as the current value, do not generate an API request
  • Doc block comments and readme update to surface these changes

Test Plan

  • Unit tests
  • Test app verification

Related Issues/Tickets

CHNL-7204

Evan Masseau added 2 commits April 11, 2024 13:20
…prone API calls

- Trim whitespace from identifiers
- Reject with warning any blank string value for identifiers
- Setting identifiers with the fluent setters to the same value as before will not enqueue an API request
@@ -58,3 +58,10 @@ sealed class ProfileKey(name: String) : Keyword(name) {
else -> name
}
}

internal val PROFILE_IDENTIFIERS = arrayOf(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to put this inside Profile.companionObject but there must be some initialization order issue, because it just came out as an array of nulls.

@evan-masseau evan-masseau marked this pull request as ready for review April 11, 2024 17:40
@evan-masseau evan-masseau requested a review from a team as a code owner April 11, 2024 17:40
Copy link
Contributor

@ajaysubra ajaysubra left a comment

Choose a reason for hiding this comment

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

LGTM

)
null
}?.also { validatedIdentifier ->
var property by when (propertyKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

neat!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right?? kotlin is cool

@evan-masseau evan-masseau merged commit ca6ebdc into master Apr 15, 2024
2 checks passed
@evan-masseau evan-masseau deleted the ecm/minimal-identifier-validation branch April 15, 2024 14:05
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.

None yet

3 participants