Skip to content
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

Fix filter APIs #1341

Closed

Conversation

vboctor
Copy link
Member

@vboctor vboctor commented Apr 21, 2018

The filter API was broken by recent refactoring causing it to return invalid id, name, public, and extra _filter_id. This impacted both SOAP and REST APIs.

Fixes #24335, #24349

@vboctor vboctor self-assigned this Apr 21, 2018
@vboctor vboctor requested a review from cproensa April 21, 2018 19:56
@@ -614,6 +614,7 @@ private function convertTypeToJson( &$p_criteria ) {
unset( $p_criteria['_version'] );
unset( $p_criteria['_source_query_id'] );
unset( $p_criteria['_view_type'] );
unset( $p_criteria['_filter_id'] );
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not familiar with the external apis.
Returning filter id is bad in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

The code here is to remove _filter_id from the criteria, since it is usually surfaced as id and it is part of the filter rather than the filter criteria.

$t_row['user_id'] = filter_get_field( $t_filter_id, 'user_id' );
$t_row['name'] = $t_filter_name;
$t_row['is_public'] = filter_get_field( $t_filter_id, 'is_public' );
$t_row['project_id'] = filter_get_field( $t_filter_id, 'project_id' );
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as i can tell, the new values added here were not part of the array in the previous code
(see 13b9aef)

Copy link
Member Author

Choose a reason for hiding this comment

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

The old code operated on a filter_row which included such fields. The new code gets the filter criteria which doesn't include such fields.

Choose a reason for hiding this comment

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

What about filter_string field? It is required in API method mc_filter_get (api/soap/mc_filter_api.php)

@vboctor
Copy link
Member Author

vboctor commented Apr 22, 2018

I'm planning to target this to master-2.13.

The filter API was broken by recent refactoring causing it to return invalid id, name, public, and extra _filter_id.
This broke both REST and SOAP APIs.

Fixes #24335, #24349
@vboctor vboctor force-pushed the issue24335_rest_fix_filter_apis branch from 8e94e3b to 9f4e219 Compare April 25, 2018 01:16
@vboctor
Copy link
Member Author

vboctor commented Apr 25, 2018

I have updated the PR to also fix the SOAP API.

@vboctor vboctor changed the base branch from master to master-2.13 April 28, 2018 00:32
@vboctor vboctor changed the base branch from master-2.13 to master April 28, 2018 00:33
@vboctor
Copy link
Member Author

vboctor commented Apr 28, 2018

Merged via b739c9f

@vboctor vboctor closed this Apr 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants