nudj / nudj-backend

Nudj - Backend (Archive)
0 stars 0 forks source link

Relax requirement to supply contact IDs when the client is sending the SMS #19

Closed richardbuckle closed 8 years ago

richardbuckle commented 8 years ago

Currently the APIs api/v1/nudge and api/v1/nudge/ask return an error if the contacts parameter is missing or is an empty array.

Please relax this requirement when the client_will_send parameter is true.

Dev only for now.

shtukas commented 8 years ago

@richardbuckle

I confirm that the following works fine

curl -X PUT \
    -d "job=12" \
    -d "contacts[]=127" \
    -d "message=testing1346" \
    --header "token:KdNS0w3pb67XrIOsxoOiRZpasJStNQyPTV7J0DkMuO1ib0OMqEM7Rql6RS4g" \
    "http://localhost:8000/api/v1/nudge"

while this one...

curl -X PUT \
    -d "job=12" \
    -d "message=testing1346" \
    --header "token:KdNS0w3pb67XrIOsxoOiRZpasJStNQyPTV7J0DkMuO1ib0OMqEM7Rql6RS4g" \
    "http://localhost:8000/api/v1/nudge"

returns

{
    "error": {
        "message": "Validation Error :: One or more fields does not meet the requirements :: {\"contacts\":[\"The contacts field is required.\"]}",
        "code": 11102
    },
    "timestamp": 1463903012.4369
}

But I propose something else.

The error message only occurs because the backend PHP code is badly written. If the contacts parameter is missing ("empty" or "missing" both mean the same as far of the HTTP request is concerned) the backend code should not return an error. I propose to update the backend code, instead of changing the semantics of the call. Tell me if you agree and I will make the change.

nb: This call only has effects if there are actually people to contacts. If the contacts parameter is empty (or missing) then the client code could as well decide just not make that call. ( This, regardless of the correction I propose to make ).

richardbuckle commented 8 years ago

I would like the new optional parameter client_will_send to govern this.

If there are any questions please ask!

shtukas commented 8 years ago

Um........., ....

Part 1

So in Laravel the validation of a request is done before I can observe it. For instance, the code rejects calls without the contacts attributes (and sends an error back at you) before I can observe whether the client_will_send attributes is set or not.

This happens because of something called NudgeRequest, that is defined elsewhere and is called from the header of that controller handler.

I had to remove it, so now, this attribute is never required. This has been done for "NudgeRequest" and "AskForReferralsRequest" which affect api/v1/nudge and api/v1/nudge/ask.

Part 2

Now, in the case of a nudge (the other case is similar), if the attribute client_will_send is present and set to "true", then a different method is run. The new method is like the original but doesn't perform the sms sending.

Deployed in Dev. Cheers,