Command and REST API improvements #1263
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Instance of data accepting payload that is sometimes updated from the input from REST API. It now works as follows: - data[‘payload’] corresponds to REST API body. - data[‘query’] corresponds to REST API URL path and query string params. - data[‘options’] options to take into consideration when executing the command from the calling module. If these elements are not set, they will be defaulted by the commands to an empty array.
- option() to access ‘options’. - query() to access ‘query’. - payload() To access ‘payload’.
- Differentiate invalid value from not found errors. - Generate proper error when users is set to a non-array value.
- Use for REST API - Use for Web UI
This is a step towards de-coupling attaching of files from the web UI post format. A follow up will eventually allow attachments being included ias base64 content.
Following is a sample attribute to pass when creating a note:
```
“files”: [
{
"name": "test.txt",
"content": "VGhpcyBpcyBhIFRFU1QuDQpUaGlzIGlzIGEgVEVTVC4NClRoaXMgaXMgYSBURVNULg0KVGhpcyBpcyBhIFRFU1QuDQpUaGlzIGlzIGEgVEVTVC4="
}
]
```
vboctor
commented
Jan 4, 2018
| "duration": "00:45", | ||
| }, | ||
| "files": [ | ||
| { |
- Check that the other user has report issue note access. - Attribute the note to the other user rather than the logged in user.
Make sure access checks are complete and are applied against the effective reporter. - Private notes - Attahcing files - Time tracking Update command sample data with comments and fixed indentation.
Base logic on reporter id rather than logged in user id.
- attr_value should only be used for reminders and doesn’t make sense to happen via SOAP API. - note type can’t be TIME_TRACKING if note doesn’t have time tracking information. - If note type not specified, but time tracking info is supplied, make sure type is set to TIME_TRACKING.
atrol
reviewed
Jan 5, 2018
| * @param string $p_file_path The file path. | ||
| * @return boolean|string The mime type or false on failure. | ||
| */ | ||
| function file_get_mime_type_for_content( $p_content ) { |
There was a problem hiding this comment.
The name of the previous function is file_get_mime_type.
Shouldn't the name of this function be content_get_mime_type ?
There was a problem hiding this comment.
Nope. file_ is the the prefix for all file_api. At the end of the day this is a file_ api that acts on file content rather than file path.
atrol
reviewed
Jan 5, 2018
core/file_api.php
Outdated
| if( is_blank( $t_info_file ) ) { | ||
| $t_finfo = new finfo( FILEINFO_MIME ); | ||
| } else { | ||
| $t_finfo = new finfo( FILEINFO_MIME, $t_info_file ); | ||
| } |
There was a problem hiding this comment.
The code block starting with $t_info_file = ... is the same like the block in function file_get_mime_type
Instead of the whole block, use existing function finfo_get_if_available.
There was a problem hiding this comment.
I just pushed a refactored version of the code.
vboctor
force-pushed
the
command_note_add
branch
from
b249f54
to
c12c72f
Compare
Get Issue and Add Note should use consistent time tracking formatting.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR restructured command data into multiple sub-arrays. These are:
query- typically used for url and query string parameters.payload- used for the payload that is expected to be passed with a POST command.options- provides the caller of the command the ability to pass options that control the command behavior.The separation provides the advantage of having better similarity with REST API and protecting against a REST API payload specifying a value that it shouldn't, e.g. the issue_id that was provided before, or an option to a command.
The commands were also updated so as not to modify the passed in data. If they are to validate, normalize, and update passed in values, then such values are shared into command properties that are shared between validate() and process() methods.
The existing command was updated to follow the new model.
This PR also includes the following features: