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

[User Model] Fixes #640

Merged
merged 7 commits into from
Sep 22, 2023
Merged

[User Model] Fixes #640

merged 7 commits into from
Sep 22, 2023

Conversation

shepherd-l
Copy link
Contributor

@shepherd-l shepherd-l commented Sep 22, 2023

Description

One Line Summary

  • Updated troubleshooting section in migration guide with More than one file was found with OS independent path 'META-INF/kotlinx_coroutines_core.version'. Android build error.
  • Updated EDM4U version from 1.2.169 to 1.2.177 in the OneSignal-Unity-SDK setup step and in the example app
  • Updated included OneSignal-Android-SDK to 5.0.1
  • Fixed sending VSAttribution data from the editor
  • Added notifications permission button in example app for easier testing

Details

Motivation

Fixes VSA data not being sent and addresses More than one file was found with OS independent path 'META-INF/kotlinx_coroutines_core.version'. Android build error.

Testing

Manual testing

Tested OneSignal initialization, app build with Unity 2021.3.16f1 of the OneSignal example app on a emulated Pixel 4 with Android 12.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

```

1. Go to **Player Settings > Android Tab > Publishing Settings** and enable the following settings:
Copy link
Member

Choose a reason for hiding this comment

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

We also need to include these steps are part of the setup guide too in the README.md.

Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

The title of the PR says "[Fix] Sending VSA data" but it does a lot more than that. Can we update this PR title?

This PR also does a lot of different things, in the future please split up PRs if they contain unrelated commits

@shepherd-l shepherd-l changed the title [Fix] Sending VSA data [User Model] Fixes Sep 22, 2023
@shepherd-l
Copy link
Contributor Author

Will do, thanks for the feedback

@shepherd-l shepherd-l requested a review from jkasten2 September 22, 2023 21:00
@shepherd-l shepherd-l merged commit 7ebbba0 into user-model/main Sep 22, 2023
1 check passed
@shepherd-l shepherd-l deleted the fix/VSA branch September 22, 2023 21:24
jinliu9508 pushed a commit that referenced this pull request Feb 23, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Feb 27, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 1, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 4, 2024
jinliu9508 pushed a commit that referenced this pull request Mar 4, 2024
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