Skip to content

Conversation

@hiranya911
Copy link
Contributor

The new FCM API does not accept topic names prefixed with /topics/. But we can easily check for this in the SDK and remove it. This results in a better and consistent dev experience since the other methods in this API accepts prefixed topic names.

this.webpushConfig = builder.webpushConfig;
this.apnsConfig = builder.apnsConfig;

String unprefixedTopic = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you move this to a helper function and call this stripPrefix()? I think the name unprefixedTopic will never win my heart over :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@hiranya911 hiranya911 merged commit b96b220 into hkj-fcm-api Feb 7, 2018
@hiranya911 hiranya911 deleted the hkj-fcm-topics branch February 7, 2018 02:05
hiranya911 added a commit that referenced this pull request Feb 13, 2018
* Adding experimental FCM API

* Added Webpush notification support

* Added APNS support and tests

* Added subscribe method

* Added TopicManagementResponse type

* Added unsub method

* Improved error handling and json parsing

* More tests, validation and dry run mode

* Improved error handling and tests

* Added license header; Implemented topic name normalization

* Improved error handling

* Improved error handling

* Added javadocs

* Fixing auth (ADC) test failure

* Updated documentation

* Updated documentation

* Updated javadocs

* Updated test

* Renamed Priority enum values (using uppercase)

* Adding APNS types

* Updated APNS documentation

* Updated integration test

* Making TTL a long argument

* Updated changelog

* Addressing a number of readability nits, and other code review comments

* Fixing some nits

* Removing some duplicate code in messaging tests

* Further deduping code in test

* Accept prefixed topic names (#136)

* Accept prefixed topic names

* Parsing topic name in a helper method

* Updated javadoc for setTopic()

* Updated FCM error codes (#138)

* Merged with dev; updated changelog

* Updated changelog
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