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

Added fix for invalid cast exception for SendTags on Android #313

Merged
merged 1 commit into from
Jan 19, 2023

Conversation

ChrisMonson
Copy link

@ChrisMonson ChrisMonson commented Jul 1, 2022

Description

One Line Summary

Implemented a fix for the InvalidCastException noted in issue #311

Details

Motivation

This update was made to fix SendTags throwing and InvalidCastException whenever it is called on Android.

Scope

This change only affects Android, and is limited to the SendTags method. I changed what was previously an attempt to cast a string to a JSONObject into using the JSONObject constructor to pass in the JSON string.

Manual testing

I tested the code on Android 12 using Visual Studio 2022 on Windows. Calling SendTags now correctly sends the dictionary of tags passed in to the OneSignal API and the tags are visible through the web interface.

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
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • 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.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is Reviewable

@ChrisMonson
Copy link
Author

Hey all, any chance someone can take a look at this? Currently we are having to make about 20 sequential calls to SendTag because SendTags is completely broken. The fix for this modifies a single line of code and doesn't touch anything else in the codebase. We'd really appreciate if this fix or another one could get applied to resolve this issue. Thank you!

@lafritay
Copy link

We're hitting this as well...

@kevin-david
Copy link
Contributor

kevin-david commented Dec 31, 2022

Thanks for the fix, @ChrisMonson. Until OneSignal merges this PR and releases a new version, I forked this SDK and got it building with a nuget package published here: https://www.nuget.org/packages/OneSignalSDK.Xamarin.kevin-david

Source: https://github.com/kevin-david/OneSignal-Xamarin-SDK

@brismithers
Copy link
Contributor

Apologies for the delay, will get this merged and shipped.

@brismithers brismithers merged commit 9f946c9 into OneSignal:main Jan 19, 2023
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