Skip to content

Transact action should support common encoding formats- JSON, Transit, edn, and Form-POST#62

Open
ohpauleez wants to merge 2 commits intomasterfrom
expand-http-param
Open

Transact action should support common encoding formats- JSON, Transit, edn, and Form-POST#62
ohpauleez wants to merge 2 commits intomasterfrom
expand-http-param

Conversation

@ohpauleez
Copy link
Copy Markdown
Contributor

@ohpauleez ohpauleez commented Mar 24, 2017

This addresses issue #45

Resolving the "payload" parameter for transact is handled by a separate function.
Currently, this function also grabs form params as a last effort (which are not nested under payload).

@ddeaguiar
Copy link
Copy Markdown
Contributor

Unrelated to this PR but I noticed that the merged-parameters fn doesn't include transit-params. Should it?

@ohpauleez
Copy link
Copy Markdown
Contributor Author

It does now :) -- nice catch.

@ddeaguiar
Copy link
Copy Markdown
Contributor

I think we'll want to add form-params to merged-parameters as well. Another thing is that the caller is expecting a collection of maps but :form-params will just be a map. AFAIK, you can't submit a collection.

@ddeaguiar
Copy link
Copy Markdown
Contributor

@ohpauleez I spent some time thinking about this PR and thought that extracting resolve-payload-parameter to an an interceptor or interceptors may provide more flexibility. I'd consider at least two interceptors here. One for json, transit and edn params and another for form params because they require some transformation, at least based on the current usage in transact-action-exprs. I'd have transact-action-exprs depend on a new params key, like :vase-params, whose contents should be a collection of txdata, which would be populated by these interceptors.

In the future this could even be made configurable so that developers could specify which content-types are supported – a consequence of this is that such information could be feed into specification generation tooling for things like Swagger.

I'd be happy to adopt this PR and hash this out if you're up for it.

@ddeaguiar
Copy link
Copy Markdown
Contributor

Alternatively, resolve-payload-parameter can stick around and support json, transit and end payloads but :form-param support could be moved to an interceptor. I think a new params key would still be needed in this case, though.

@ddeaguiar
Copy link
Copy Markdown
Contributor

Ping @ohpauleez just following up. This PR is related to issue #78 as well.

@CalebMacdonaldBlack
Copy link
Copy Markdown
Contributor

I'd really see this PR make it in to send edn instead of json

@ddeaguiar
Copy link
Copy Markdown
Contributor

@ohpauleez I'm guessing this work is sitting on the back burner. Would it be ok if I pushed it forward?

@ddeaguiar ddeaguiar added the 0.9.4 Include in 0.9.4 release label May 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.9.4 Include in 0.9.4 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants