W-16451172 Wr/api request rest #5
Conversation
mshanemc
left a comment
mshanemc
left a comment
There was a problem hiding this comment.
pretty sure the file-body isn't working. I couldn't get the --body - trick to work, either.
Maybe that's another point in favor of making --file and --body separate flags?
src/commands/api/request/rest.ts
Outdated
| // TODO: getting a false positive from this eslint rule. | ||
| // summary is already set in the org flag. | ||
| // eslint-disable-next-line sf-plugin/flag-summary | ||
| 'target-org': Flags.requiredOrg({ | ||
| helpValue: 'username', | ||
| }), |
There was a problem hiding this comment.
| // TODO: getting a false positive from this eslint rule. | |
| // summary is already set in the org flag. | |
| // eslint-disable-next-line sf-plugin/flag-summary | |
| 'target-org': Flags.requiredOrg({ | |
| helpValue: 'username', | |
| }), | |
| 'target-org': Flags.requiredOrg(), |
There was a problem hiding this comment.
the suggestion is works ok. I think the help output is fine without the override. Was there something that bothered you about the flag's defaults?
There was a problem hiding this comment.
just dropped this in from Cristian's plugin 🤷♂️
There was a problem hiding this comment.
yeah, probably something wrong in my plugin or was fixed in latest deps (mine is a bit stale)
src/commands/api/request/rest.ts
Outdated
| }), | ||
| }; | ||
|
|
||
| private static getHeaders(keyValPair: string[]): Headers { |
There was a problem hiding this comment.
should be a function and not a static method so its testable
There was a problem hiding this comment.
and will likely be useful in other commands!
There was a problem hiding this comment.
another benefit of having a fn is that you could put it as the parse prop on the --header flag so that if people have bad headers it fails sooner (avoiding org re-auth, etc)
src/commands/api/request/rest.ts
Outdated
|
|
||
| await org.refreshAuth(); | ||
|
|
||
| const url = `${org.getField<string>(Org.Fields.INSTANCE_URL)}/${args.endpoint}`; |
There was a problem hiding this comment.
is there anything in rest that's not in services/data/vXX.0 ? If not, why not default those (or use Connection) and only make people type the remaining url fragments?
There was a problem hiding this comment.
another idea: passing this to Url constructor might do a nice validation that what they typed for endpoint, when combined with the instance, is a valid URL (and maybe do THAT before org re-auth)
There was a problem hiding this comment.
yeah, there's opinions on how similar it should be to curl - that's a good point, I'm not sure I'd be annoyed if I would have to type that out every time, or think it makes sense... it's in "beta" so we'll be able to gather feedback and adjust
|
QA notes: ✅ bunch of GETs against the various endpoints, including sobject record used below 🎨 it's not obvious, given the addition of the 🎨 this does what I think it should to the record in the org [in that example, there is no response body, but it's not clear that that's actually the case from the command. Maybe a placeholder or warning that there is no body? I can see the empty logline but wouldn't have guessed that was what was happening]
✅ --stream-to-file |
What does this PR do?
adds
api request restcommand to send API requests to an orgcloned from Cristian's plugin
in
betastate for feedback periodWhat issues does this PR fix or reference?
@W-16451172@