New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REST and SOAP API Improvements #1100

Merged
merged 27 commits into from May 4, 2017

Conversation

Projects
None yet
2 participants
@vboctor
Member

vboctor commented Apr 23, 2017

It is recommended to review this PR one commit at a time to follow the chain of changes.

REST API

  • Support getting multiple issues with pagination. This can be based on filter or project. Whether returning a single issue (by id) or a batch of issues, the same response format is now used which has issues array.
  • Setup framework for testing REST API + tests for issue creation.
  • When time tracking is not visible, return note type as note rather than timelog if it has a hidden time.
  • Return versions as an object with id and name rather than just a string.
  • Enum name should be the English (from config) value.
  • Added enum label which has the localized version of the enum name.
  • Fix some error messages that were not correctly reflecting the error case.
  • Include status color as part of status info on issues.
  • Use created_at rather than date_submitted.
  • Use updated_at rather than last_updated.
  • Make sticky a boolean rather than a string.
  • Don't include sponsorship_total.

SOAP and REST

  • Added some config option checks that were missing from SOAP/REST API (e.g. eta enabled, projection enabled, profile, build/platform/os, etc).
  • Don't allow setting an undefined version when creating an issue.
  • Do access checks based on bug/project rather than global access level for fields like due date.

vboctor added some commits Apr 22, 2017

Update issues formatting in REST API
- Use `created_at` rather than ‘date_submitted`
- Use `updated_at` rather than `last_updated`.
- Make `sticky` a boolean rather than a numeric string.
- Don’t include `sponsorship_total`
REST: Improve formatting of notes
- If `attr` is empty don’t include it.
- If note is not of type `timelog` don’t return time_tracking minutes.
REST: Normalize issue when retrieving 1 or many
- Instead of having different code for preparing a single vs. multiple issue.  Use one.
- Use an object rather than a string for version, target version, and fixed in version.
- Always check that user has access to view handler.
REST: Extend get issue to return 1 or many
- Use consistent response payload when returning 1 or many.  This changed the
payload when getting a single issue.
- Support pagination when returning multiple issues.
- Support returning multiple issues based on a `project_id`.
- Support returning multiple issues based on a filter id + `project_id`.
REST: `mci_issue_data_as_array()` code cleanup
- Fix indentation
- Fix some ordering (more to do later).
REST: enum localization fixes
- Enum name should reflect english enum name.
- Fix php error due to referencing enum by localized name.
- Remove `mci_get_enum_element()` which is not needed.
REST: Implement `mci_get_version()` and use it
- Implement `mci_get_version()` similar to `mci_get_category()`
- Move `mci_get_category()` from `mc_issue_api.php` to `mc_api.php`.
- Change `mci_get_category()` to return null instead of array( ‘category’ => null )
in case of REST when category is not set.
- For SOAP if category is not set, return null from `mci_get_category()`.
API: Use due_date access check based on project
When creating/updating issues, check user project access level rather than global.
REST: Include version id in version references
Instead of just returning the name, also return the version id.
This is consistent with other fields like category, priority, etc.
If version is not found, return null.  This also applies for SOAP API.
REST: Mark `timelog` as `note` if time is hidden
If time tracking is not accessible, then return the note type as `note` instead of `timelog`.
SOAP: Fix unit tests that were based old behavior
- Don’t expect ETA fields if not enabled
- Don’t expect Project fields if not enabled
- Don’t expect undefined versions to be set on issue.
- `testUpdateWithRareFields()` was being skipped when time tracking is disabled, though it is unrelated.
- Don’t test profile/build/platform fields if disabled.
- Have default timestamp in bootstrap file to avoid failures in some dev/test envs.
REST: Test cases for adding issues
- Several test cases for valid/invalid create issue request.
- Test cases don’t run by default until bootstrapping is supported via a script.
REST: Use Guzzle for Testing
- Much cleaner way to author http requests
- Easier to check status codes
- Can work without CURL

@vboctor vboctor self-assigned this Apr 24, 2017

@rombert

This comment has been minimized.

Show comment
Hide comment
@rombert

rombert Apr 24, 2017

Member

Test cases look good at a first glance to me ( did not test ).

Have you considered sharing the test cases between the SOAP API and the REST API, while leaving the test setup separated? That would quickly increase the coverage of the REST API and we would only maintain one test suite in the future.

In the areas in which the two APIs differ - e.g. SOAP API not getting new features - we can easily find a way to only run some checks for one of the APIs.

Member

rombert commented Apr 24, 2017

Test cases look good at a first glance to me ( did not test ).

Have you considered sharing the test cases between the SOAP API and the REST API, while leaving the test setup separated? That would quickly increase the coverage of the REST API and we would only maintain one test suite in the future.

In the areas in which the two APIs differ - e.g. SOAP API not getting new features - we can easily find a way to only run some checks for one of the APIs.

@vboctor

This comment has been minimized.

Show comment
Hide comment
@vboctor

vboctor Apr 24, 2017

Member

@rombert Thanks for the review.

Have you considered sharing the test cases between the SOAP API and the REST API, while leaving the test setup separated? That would quickly increase the coverage of the REST API and we would only maintain one test suite in the future.

I haven't considered sharing test cases and I'm not in favor of it for the following reasons:

  • As I'm churning the shared code, it would be good to at least have a stable non-shared test cases for SOAP to validate. Churning both won't help.
  • The API (capabilities), their requests, their responses, etc are likely to be different. Hence, there will be a lot of if SOAP vs. REST even when validating the results. There may be even gaps in functionality in one vs. the other.
  • Test cases are often used as samples for using an API. Hence, readability of such test cases is important. They will also serve as example for plugins, so the simpler the better.
  • I expect less churn in SOAP and its associated test cases. So hopefully not much maintenance there until it is fully retired.
Member

vboctor commented Apr 24, 2017

@rombert Thanks for the review.

Have you considered sharing the test cases between the SOAP API and the REST API, while leaving the test setup separated? That would quickly increase the coverage of the REST API and we would only maintain one test suite in the future.

I haven't considered sharing test cases and I'm not in favor of it for the following reasons:

  • As I'm churning the shared code, it would be good to at least have a stable non-shared test cases for SOAP to validate. Churning both won't help.
  • The API (capabilities), their requests, their responses, etc are likely to be different. Hence, there will be a lot of if SOAP vs. REST even when validating the results. There may be even gaps in functionality in one vs. the other.
  • Test cases are often used as samples for using an API. Hence, readability of such test cases is important. They will also serve as example for plugins, so the simpler the better.
  • I expect less churn in SOAP and its associated test cases. So hopefully not much maintenance there until it is fully retired.

vboctor added some commits Apr 27, 2017

REST: Handle version as string or object
- Refactor code for validating and looking up version in db.
- Re-use the same logic for version, fixed_in_version and target_version.
- Support version as string, object with id, object with name, object with id and name (id wins).
- Don’t set invalid versions, default or error based on configuration.
- Added test cases for versions.
REST: Tag test cases and fixes
- Avoid 5xx errors when tag id not set.
- Support tag specified by name field in tag object.
- Return 404 when a tag is not found.
- Add test cases for tags
REST: users/me shouldn’t return disabled projects
When returning the list of projects the user has access to, don’t include disabled ones.
REST: use std transformation for user/proj enums
Use the standard enum transformation that returns id, name, and localized label.
REST: Add support for getting all projects
- Add ability to get all projects accessible to user including
proj info, custom fields, categories and versions.
- Change users/me to return just accessible project ids and names, and not full info.

@vboctor vboctor merged commit 4fd21d0 into mantisbt:master May 4, 2017

@vboctor vboctor deleted the vboctor:rest_api_improvements branch Sep 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment