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

Merge Params with Body in API POST requests #278

Merged
merged 1 commit into from
Jul 24, 2022

Conversation

shbatm
Copy link
Contributor

@shbatm shbatm commented Jun 21, 2022

Bug Fix

API resources for module control and notifications allow for POST requests, but ignore the body of the request, so they don't actually work. This merges the body and params objects to allow POST requests to work properly for these two endpoints.

Steps to reproduce

Attempt to call a POST request with only body contents containng the moduleName and action to /api/module and you will get the default reponse of listing all modules' info. After the change, it will correctly call the action.

Additional info

Note this is required to allow creation of Home Assistant REST Switches to work properly for showing/hiding modules. These require the URL to be the same and only the body to change. Currently this doesn't work because you have to use GET requests to different urls.

@ezeholz
Copy link
Collaborator

ezeholz commented Jun 23, 2022

Hey! Thank you so much for your contribution!
However, I believe that this way isn't the cleanest when we talk about API. We should grab both params and body separately, and later in the code compare if exist any inside those two.
Let me see what I can do to fix it. :D

@shbatm
Copy link
Contributor Author

shbatm commented Jun 23, 2022

I was trying to keep it as simple as possible, the params are expanded first to use as much of the URL as is provided (e.g. enabling sending the module name in either URL or body) and then merging in the body variables.

If the user provides a variable in both, body will take precidence.

The answerXApi functions already handle the rest of the processing.

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had
recent activity. It will be closed if no further activity occurs. Thank you
for your contributions.

@github-actions github-actions bot added the stale label Jul 24, 2022
@ezeholz ezeholz changed the base branch from master to bug-fixes July 24, 2022 02:17
@ezeholz ezeholz merged commit 67697e4 into Jopyth:bug-fixes Jul 24, 2022
@shbatm shbatm deleted the patch-1 branch July 24, 2022 08:04
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.

2 participants