Suppression list Insert and Delete #40
Conversation
lib/suppression_list.ex
Outdated
| Insert or update a single entry in the suppression list. | ||
| Returns a single string with the success message if the entry | ||
| was updated or inserted. Returns a %SparkPost.Endpoint.Error{} with a 400 | ||
| if the type is invalid. |
There was a problem hiding this comment.
"...if there was an issue with the request format" ?
lib/suppression_list.ex
Outdated
| - type: one of "transactional" or "non_transactional" | ||
| - description (optional): optional description of this entry in the suppression list | ||
| """ | ||
| def update_one(recipient, type, description \\ nil) do |
There was a problem hiding this comment.
Think we should call it insert_or_update_one/3 or upsert_one/3?
| else | ||
| %{type: type, description: description} | ||
| end | ||
| response = Endpoint.request(:put, "suppression-list/#{recipient}", body) |
There was a problem hiding this comment.
I wonder if we should adopt the usual {:ok, %Sparkpost.Endpoint.Response{}} or {:error, %SparkPost.Endpoint.Error{}} here.
There was a problem hiding this comment.
I'd like to adopt this. Unfortunately I'm not sure I have enough time to go through and update the library everywhere to make sure everything still works. Might make sense to have this in a different PR?
There was a problem hiding this comment.
Yup. I wasn't suggesting you retrofit the whole library with this. These are the first endpoints with simple good/bad responses though so I thought we'd start here.
| Parameters: | ||
| recipient: the entry to delete from the suppression list. | ||
| """ | ||
| def delete(recipient) do |
There was a problem hiding this comment.
The same comment about idiomatic {:ok, ...} return values applies here.
| decoded_body = decode_response_body(body) | ||
| if decode_results do | ||
| %SparkPost.Endpoint.Response{status_code: 200, results: decoded_body.results} | ||
| %SparkPost.Endpoint.Response{status_code: code, results: decoded_body.results} |
There was a problem hiding this comment.
Will have to check which other modules either make assumptions about the status code or pass it on to client code. This could produce a (good) breaking change in those cases.
There was a problem hiding this comment.
didn't see anything on a cursory glance. Let me know if I missed anything though.
…tic {:ok | :error, resp} tuples
|
@ewandennis started the migration over to the idiomatic |
|
Sorry for the delay on this. I'm going to merge, do a little cleanup elsewhere and cut a release. Thanks as ever @asgoel ! |
3 participants
@ewandennis added ability to insert/update or delete a single suppression list entry.
Also had to add one more error check to the endpoint response parser to handle normal HTTPoison errors (such as request timeouts).