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

fix: add attributes to properties #649

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

ami-aman
Copy link
Contributor

Based on sample app testing it was found that the device attributes are not passed in properties parameters but context.device. This PR fixes the issue!

Complete each step to get your pull request merged in. Learn more about the workflow this project uses.

  • Assign members of your team to review the pull request.
  • Wait for pull request status checks to complete. If there are problems, fix them until you see that all status checks are passing.
  • Wait until the pull request has been reviewed and approved by a teammate
  • After code reviews are approved and you determine this PR is ready to merge, select Squash and Merge button on this screen, leave the title and description to the default values, then merge the PR.

@ami-aman ami-aman requested a review from a team March 19, 2024 11:44
Copy link

github-actions bot commented Mar 19, 2024

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.


  • CocoaPods-FCM: aman/migration-device-attribute (1710850592)
  • APN-UIKit: aman/migration-device-attribute (1710850573)

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 53.24%. Comparing base (9996872) to head (4c0d0c3).
Report is 122 commits behind head on main-replica-for-cdp.

❗ Current head 4c0d0c3 differs from pull request most recent head 46c7b80. Consider uploading reports for the commit 46c7b80 to get more accurate results

Files Patch % Lines
...rces/DataPipeline/DataPipelineImplementation.swift 0.00% 2 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           main-replica-for-cdp     #649      +/-   ##
========================================================
- Coverage                 57.35%   53.24%   -4.12%     
========================================================
  Files                       132      139       +7     
  Lines                      3780     3899     +119     
========================================================
- Hits                       2168     2076      -92     
- Misses                     1612     1823     +211     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Shahroz16 Shahroz16 left a comment

Choose a reason for hiding this comment

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

Looks good, I am assuming test cases are going to follow in the next PR.

@ami-aman
Copy link
Contributor Author

Looks good, I am assuming test cases are going to follow in the next PR.

Thanks for reviewing, Shahroz! Test cases will be covered under this ticket.

@ami-aman ami-aman merged commit 4b02e92 into main-replica-for-cdp Mar 19, 2024
6 of 8 checks passed
@ami-aman ami-aman deleted the aman/migration-device-attribute branch March 19, 2024 12:56
github-actions bot pushed a commit that referenced this pull request Mar 20, 2024
## [3.0.0](2.12.5...3.0.0) (2024-03-20)

### ⚠ BREAKING CHANGES

* iOS as a source for Data Pipelines (#659)

### Features

* iOS as a source for Data Pipelines ([#659](#659)) ([0a68373](0a68373))
* migration module to cater to all migration tasks  ([#530](#530)) ([2feb1d4](2feb1d4))

### Bug Fixes

* add attributes to properties  ([#649](#649)) ([4b02e92](4b02e92))
* all sdk modules can only be initialized once ([ae46c7f](ae46c7f))
* app crash on identify method ([#458](#458)) ([13e9862](13e9862))
* compilation for test ([f14b773](f14b773))
* compilation issue ([420a61e](420a61e))
* eventbus handler ref ([#469](#469)) ([8c8ef91](8c8ef91))
* journey id in migration payload ([#653](#653)) ([3b649c9](3b649c9))
* prevent duplicate automatic screenview events from being tracked ([fea9ec5](fea9ec5))
* pushEventHandler test ([dc80fc2](dc80fc2))
* remove occurrence of autoTrackDeviceAttributes from all push modules ([#505](#505)) ([8dc6507](8dc6507))
* removed last_used from properties  ([#477](#477)) ([b0b9631](b0b9631))
* sample app issues ([#551](#551)) ([05544b3](05544b3))
* use git commit instead of git branch for segment dependency ([d245015](d245015))
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