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

Make device attribute optional for subscription requests #191

Conversation

akoshunyadi
Copy link
Collaborator

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

The device attribute in the subscription request has been made optional.

Which issue(s) this PR fixes:

Fixes #189

Special notes for reviewers:

Changelog input

Make device attribute optional for subscription requests

Additional documentation

@bigludo7
Copy link
Collaborator

Hi @akoshunyadi
I'm a bit uncomfortable on this one as we must:

  • in the notification send back the device identifier
  • in the Subscription resource itself have the device identifier in the GET answer (or in POST response)

If we make it optional in the request, it means that the API server will have to get it from the access token and then store it in the subscription resource.
I tend to think that keeping it mandatory makes the thing easier - wdyt?

@akoshunyadi
Copy link
Collaborator Author

@bigludo7 yes, that's a good point! I don't think that we should expose the phone-number from the access token either.

@fernandopradocabrillo
Copy link
Collaborator

@akoshunyadi I agree with @bigludo7 on this one, I also prefer to have it required

@akoshunyadi
Copy link
Collaborator Author

As decided in the last sub-project status call, the PR can be closed, see previous comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Device attribute optional for roaming and reachability subscriptions
3 participants