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

Fixed create push notification with external_id #74

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GizemSever
Copy link

@GizemSever GizemSever commented Jan 3, 2024

Description

  • Fixed create push notification with external_id, onesignal_id

Details

  • createNotification function is not send include_aliases object with extearnal_id values

Motivation

  • Fixes a specific bug,

Testing

Manual testing

  • Creating push notifications using external_id value was tested.

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 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.

@ayokay123
Copy link

Any updates?

@ansh
Copy link

ansh commented Jan 31, 2024

What is the update on this? This literally does not work. What a joke.

@nan-li
Copy link
Contributor

nan-li commented Jan 31, 2024

Hi @GizemSever, thank you for submitting this PR and include_aliases is indeed not working as expected.

We are working on overhauling the APIs and interfaces to be updated and correct, and the fix for include_aliases will be included.

In the meantime, it is still safe to use include_external_user_ids to send notifications to target by External ID, and include_player_ids to target by Subscription ID, instead of using include_aliases.

This library is actually generated from a primary spec via the OpenAPI generator so no code in this library is updated manually. In addition, include_aliases needs to be more generic since developers can have any string as an alias label, not just "onesignal_id" or "external_id".

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.

6 participants