Skip to content

Conversation

@jpastoor
Copy link
Collaborator

  • Client side cache just like getStatuses, getPriorities etc
  • Removed non-existing parameter in docblock
  • Added unittest

Fixes #122

src/Jira/Api.php Outdated
$result = $this->api(self::REQUEST_GET, '/rest/api/2/resolution', array());

foreach ( $result->getResult() as $k => $v ) {
$resolutions[$v['id']] = $v;
Copy link
Member

@aik099 aik099 Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do this:

  1. remove $k variable, because array key isn't being used
  2. rename $v variable into $resolution_data
  3. instead of doing $result->getResult() here you can specify extra argument to the ->api( call for it to return array directly back to you

Added unit tests for other local caches as well
Made a helper function to clear the local caches when a new endpoint is selected
src/Jira/Api.php Outdated
* Client-side cache of Fields.
*
* @var array
* @var array|null List of fields when loaded, null when nothing is fetched yet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In multi-line notation of @var DocBlock the description should go in dedicated place (up). In single line @var DocBlock notation indeed the description would be written right after type name.

@jpastoor jpastoor closed this Aug 18, 2016
@jpastoor jpastoor deleted the improvement/122-GetResolutionsCache branch August 18, 2016 11:01
@aik099
Copy link
Member

aik099 commented Aug 18, 2016

PR branch was deleted. Are you force pushing maybe?

@jpastoor jpastoor restored the improvement/122-GetResolutionsCache branch August 18, 2016 11:08
@jpastoor jpastoor reopened this Aug 18, 2016
src/Jira/Api.php Outdated

/**
* Fields.
* Client-side cache of Fields. List of fields when loaded, null when nothing is fetched yet.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Fields should be in lowercase. It was capitalized because it was 1st word in sentence.

src/Jira/Api.php Outdated

/**
* Statuses.
* Client-side cache of Statuses. List of statuses when loaded, null when nothing is fetched yet.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Statuses should be in lowercase. It was capitalized because it was 1st word in sentence.

@aik099
Copy link
Member

aik099 commented Aug 18, 2016

Code looks good. Just noticed, that CHANGELOG.md wasn't updated to mention:

  1. change to Api::getResolutions method to support caching
  2. reset of local caches on endpoint url change for existing Api class instance

CHANGELOG.md Outdated
- Minimal supported PHP version changed from 5.2 to 5.3 by [@chobie].
- The `Api::getPriorties` renamed into `Api::getPriorities` (former method kept for BC reasons) by [@josevh].
- Remove trailing slash from endpoint url by [@Procta].
- Clearing local caches (statuses, priorities, fields and resolutions) on endpoint change [@jpastoor].
Copy link
Member

@aik099 aik099 Aug 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bugfix (although nobody reported such bug) and needs to be moved to Fixes section.

@aik099 aik099 merged commit 829724f into console-helpers:master Aug 18, 2016
@aik099
Copy link
Member

aik099 commented Aug 18, 2016

Merging. Thanks, @jpastoor .

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants