Skip to content

Simplify filter logic #7

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.

Sign up for GitHub

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

Open
smartacus wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Simplify filter logic #7

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43d59ad
Simplify filter logic
smartacus Aug 7, 2018
cc8c011
Fix URL in standard event generation
smartacus Aug 10, 2018
e6a0881
Always return array when parsing custom_display and custom_filter
smartacus Aug 11, 2018
5c73a55
Move event_type_id out of the filter list
smartacus Nov 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
328 changes: 208 additions & 120 deletions civievent-widget.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,134 +207,45 @@ public function widget( $args, $instance ) {

if ( version_compare( $this->_civiversion, '4.3.alpha1' ) < 0 ) { return; }

$fields = $this->getFields();
$customDisplay = $this->validCustomDisplayFields( $instance );
$customDisplayFields = array_intersect_key( self::getCustomDisplayTitles(), $customDisplay );

$defaultParams = array(
'start_date' => array( '>=' => date( 'Y-m-d' ) ),

@twomice twomice Mar 19, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@agh1 Do you still have concerns about down-to-the-second vs same-day calculations as in this context?

'is_public' => 1,
'options' => array(
'sort' => 'start_date ASC',
'limit' => CRM_Utils_Array::value( 'limit', $instance, 5 ),
),
);
$eventTypeIdParams = $this->buildEventTypeIdParams( $instance );
$customFilterParams = $this->validCustomFilterFields( $instance );
$returnParams = $this->buildReturnParams( $customDisplay, $customDisplayFields );
$filterParams = array_merge_recursive( $defaultParams, $eventTypeIdParams, $customFilterParams, $returnParams );

@twomice twomice Mar 19, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Using array_merge_recursive() is problematic here because it's not smart enough to know when to merge and when to replace. For example:



$defaultParams = array(
  'start_date' => array('>=' => '2019-03-19'), // hard-coding today's date for example purposes.
  // ... omitting other params for simplicity ...
);
$customFilterParams = array(
  'start_date' => array('>=' => '2020-01-01'),
);
var_dump(array_merge_recursive($defaultParams, $customFilterParams));
/* Ouput:
 array(1) {
  'start_date' =>
  array(1) {
    '>=' =>
    array(2) {
      [0] =>
      string(10) "2019-03-19"
      [1] =>
      string(10) "2020-01-01"
    }
  }
}
 * This will cause civicrm api to ignore the 'start_date' param entirely.
*/
$customFilterParams = array(
  'start_date' => array('=' => '2020-01-01'),
);
var_dump(array_merge_recursive($defaultParams, $customFilterParams));
/* Ouput:
array(1) {
  'start_date' =>
  array(2) {
    '>=' =>
    string(10) "2019-03-19"
    '=' =>
    string(10) "2020-01-01"
  }
}
 * This will cause civicrm api to return events matching EITHER of the 
 * defined 'start_date' criteria.
*/

I did have some hope for array_replace_recursive(), but it's also not smart enough, and will also result in weird combinations.

I think the only way to overcome this is with an additional method to merge these parameters explicitly to get the intended parameters.

@twomice twomice Mar 21, 2019

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

OK, actually, I think there's hope for simply array_merge() here. It resolves the above-mentioned issues, but not sure if it causes other issues.


$standardDisplay = false;
if ( ! empty( $instance['custom_display'] ) && CRM_Utils_Array::value( 'admin_type', $instance ) === 'custom' ) {
// Get the custom display params.
$custom = json_decode( $instance['custom_display'], true );
foreach ( $custom as $name => $fieldAttrs ) {
// Make sure only legit fields are sent.
if ( empty( $fields[ $name ] ) ) {
unset( $custom[ $name ] );
}
}
if ( empty( $custom ) ) {
$standardDisplay = true;
} else {
// Get custom filters.
$customFilters = json_decode( CRM_Utils_Array::value( 'custom_filter', $instance, '' ), true );
$filterParams = array(
'start_date' => array( '>=' => date( 'Y-m-d' ) ),
'is_public' => 1,
'options' => array(
'sort' => 'start_date ASC',
'limit' => CRM_Utils_Array::value( 'limit', $instance, 5 ),
),
);
$allCustomDisplayFields = self::getCustomDisplayTitles();
// Set filter params only if they're legit fields or options.
if ( is_array( $customFilters ) ) {
foreach ( $customFilters as $name => $val ) {
if ( 'custom' === $name ) {
foreach ( $val as $option => $optionVal ) {
if ( in_array( $option, $okOptions ) ) {
switch ( $option ) {
case 'limit':
case 'offset':
case 'sort':
$filterParams['options'][ $option ] = $optionVal;
}
}
}
} elseif ( array_key_exists( $name, $fields ) && ! array_key_exists( $name, $allCustomDisplayFields ) ) {
$filterParams[ $name ] = $val;
}
}
}
$fieldsToRetrieve = array_keys( $custom );
$customDisplayFields = array_intersect_key( $allCustomDisplayFields, $custom );
foreach ( $customDisplayFields as $customDisplayField => $dontcare ) {
$fieldsToRetrieve = array_merge( $fieldsToRetrieve, self::getCustomDisplayField( $customDisplayField ) );
}
// Return fields should be based on the custom_display only.
$filterParams['return'] = array_unique( $fieldsToRetrieve );
try {
$eventsCustom = civicrm_api3( 'Event', 'get', $filterParams );
if ( ! empty( $eventsCustom['values'] ) ) {
$content = '<div class="civievent-widget-list civievent-widget-custom-display">';
$index = 0;
foreach ( $eventsCustom['values'] as $eventId => $event ) {
$oe = ($index&1) ? 'odd' : 'even';
$content .= "<div class=\"civievent-widget-event civievent-widget-event-$oe civievent-widget-event-$index\">";
$index++;
foreach ( $custom as $name => $fieldAttrs ) {
if ( empty( $event[ $name ] ) ) {
if ( array_key_exists( $name, $customDisplayFields ) ) {
$fieldVal = self::getCustomDisplayField( $name, $event );
} else {
continue;
}
} else {
$fieldVal = $event[ $name ];
}
$rowField = empty( $fieldAttrs['prefix'] ) ? '' : wp_filter_kses( $fieldAttrs['prefix'] );
if ( ! empty( $fieldAttrs['title'] ) ) {
$rowField .= empty( $fieldAttrs['wrapper'] ) ? "{$fields[ $name ]}: " : "<span class=\"civievent-widget-custom-label\">{$fields[ $name ]}: </span>";
}
$rowField .= empty( $fieldAttrs['wrapper'] ) ? $fieldVal : "<span class=\"civievent-widget-custom-value\">$fieldVal</span>";
$rowField .= empty( $fieldAttrs['suffix'] ) ? '' : wp_filter_kses( $fieldAttrs['suffix'] );

$rowClass = sanitize_html_class( "civievent-widget-custom-display-$name" );
$content .= empty( $fieldAttrs['wrapper'] ) ? "$rowField\n" : "<span class=\"$rowClass\">$rowField</span>\n";
}
$content .= '</div>';
}
$content .= '</div>';
$standardDisplay = empty( $customDisplay );

try {
$customDisplayClass = $standardDisplay ? '' : ' civievent-widget-custom-display';
$content = "<div class=\"civievent-widget-list$customDisplayClass\">";
$eventsCustom = civicrm_api3( 'Event', 'get', $filterParams );
if ( ! empty( $eventsCustom['values'] ) ) {
$index = 0;
foreach ( $eventsCustom['values'] as $eventId => $event ) {
if ( $standardDisplay ) {
$content .= $this->standardEvent( $event, $instance, $index );
} else {
$content = '';
$content .= $this->customEvent( $event, $customDisplay, $customDisplayFields, $index );
}
} catch (CiviCRM_API3_Exception $e) {
CRM_Core_Error::debug_log_message( $e->getMessage() );
$index++;
}
$content .= '</div>';
}
} else {
$standardDisplay = true;
} catch (CiviCRM_API3_Exception $e) {
CRM_Core_Error::debug_log_message( $e->getMessage() );
}

if ( $standardDisplay ) {
// Outputs the content of the widget.
// apply event type filter on standard output.
$event_type_id = empty( $instance['event_type_id'] ) ? null : intval( $instance['event_type_id'] );
$cal = CRM_Event_BAO_Event::getCompleteInfo( null, $event_type_id );
$index = 0;
$content = '<div class="civievent-widget-list">';
foreach ( $cal as $event ) {
$url = CRM_Utils_Array::value( 'url', $event );
$title = CRM_Utils_Array::value( 'title', $event );
$summary = CRM_Utils_Array::value( 'summary', $event, '' );
$row = $this->dateFix( $event, 'civievent-widget-event' );
if ( $title ) {
$row .= ' <span class="civievent-widget-event-title">';
$row .= self::locFix( $event, $event['event_id'], $instance, 'civievent-widget-event' );
$row .= '<span class="civievent-widget-infolink">';
$row .= ($url) ? "<a href=\"$url\">$title</a>" : $title;
$row .= '</span>';

$row .= self::regFix( $event, $event['event_id'], 'civievent-widget' );

if ( $instance['summary'] ) {
$row .= "<span class=\"civievent-widget-event-summary\">$summary</span>";
}
$row .= '</span>';
}

$oe = ($index&1) ? 'odd' : 'even';
$content .= "<div class=\"civievent-widget-event civievent-widget-event-$oe civievent-widget-event-$index\">$row</div>";
$index++;
if ( $index >= $instance['limit'] ) { break; }
}
$content .= '</div>';
if ( $instance['alllink'] ) {
$viewall = CRM_Utils_System::url( 'civicrm/event/ical', 'reset=1&list=1&html=1' );
$content .= "<div class=\"civievent-widget-viewall\"><a href=\"$viewall\">" . ts( 'View all' ) . '</a></div>';
Expand Down Expand Up @@ -367,6 +278,183 @@ public function widget( $args, $instance ) {
}
}

/**
* Parse the "custom_display" JSON and return list of valid entries
*
* @param array $instance
* The widget instance.
* @return array
* The valid "custom_display" entries.
*/
protected function validCustomDisplayFields ( $instance ) {
$fields = $this->getFields();
$customDisplay = json_decode( empty( $instance['custom_display'] ) ? '{}' : $instance['custom_display'], true );

@twomice twomice Mar 21, 2019 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As in #7 (comment), we need to wrap this in html_entity_decode().

foreach ( $customDisplay as $name => $fieldAttrs ) {
// Make sure only legit fields are sent.
if ( empty( $fields[ $name ] ) ) {
unset( $customDisplay[ $name ] );
}
}
return $customDisplay;
}

/**
* Parse the "custom_filter" JSON and return list of valid entries
*
* @param array $instance
* The widget instance.
* @return array
* The valid "custom_filter" entries.
*/
protected function validCustomFilterFields ( $instance ) {
$fields = $this->getFields();
$allCustomDisplayFields = self::getCustomDisplayTitles();
$customFilter = json_decode( empty ( $instance['custom_filter'] ) ? '{}' : $instance['custom_filter'], true );

@twomice twomice Mar 21, 2019 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think we have to wrap this in html_entity_deccode(). This is because you can't use actual angle-brackets for things like "greater-than-or-equal-to"; normally this operator would be >=, but if you put that in the shortcode, it breaks the page. (See https://codex.wordpress.org/Shortcode_API#HTML which says "The suggested workaround for HTML limitations is to use HTML encoding for all user input, and then add HTML decoding in the custom shortcode handler."). So users need to use entities, e.g., &gt;=, and we need to run json-based attributes like custom_display through html_entity_decode() to support these operators.

foreach ( $customFilter as $name => $fieldAttrs ) {
// Make sure only legit fields are sent.
if ( $name === 'options' ) {
foreach ( $customFilter[ $name ] as $option => $optionValue ) {
if ( $option !== 'limit' && $option !== 'offset' && option !== 'sort' ) {
unset( $customFilter[ $name ][ $option ] );
}
}
} else if ( empty( $fields[ $name ] ) || array_key_exists( $name, $allCustomDisplayFields ) ) {
unset( $customFilter[ $name ] );
}
}
return $customFilter;
}

/**
* Generate filter params for the "event_type_id" setting
*
* @param array $instance
* The widget instance.
* @return array
* Filter params representing the "event_type_id" setting.
*/
protected function buildEventTypeIdParams ( $instance ) {
if ( empty( $instance['event_type_id'] ) ) {
return array();
} else {
return array(
'event_type_id' => intval( $instance['event_type_id'] )
);
}
}

/**
* Generate return params for the event query
*
* @param array $customDisplay
* The list of custom display entries.
* @param array $customDisplayFields
* The list of custom display fields.
* @return array
* Return params for the event query.
*/
protected function buildReturnParams ( $customDisplay, $customDisplayFields ) {
if ( empty( $customDisplay ) ) {
return array(
'return' => array(
'title',
'summary',
'start_date',
'end_date',
'is_online_registration',
'registration_start_date',
'registration_end_date',
'registration_link_text',
),
);
} else {
$fieldsToRetrieve = array_keys( $customDisplay );
foreach ( $customDisplayFields as $customDisplayField => $dontcare ) {
$fieldsToRetrieve = array_merge( $fieldsToRetrieve, self::getCustomDisplayField( $customDisplayField ) );
}
return array( 'return' => array_unique( $fieldsToRetrieve ) );
}
}

/**
* Generate HTML representation of a custom event
*
* @param array $event
* The event details.
* @param array $customDisplay
* The list of custom display entries.
* @param array $customDisplayFields
* The list of custom display fields.
* @param integer $index
* The index of this event.
* @return string
* An HTML representation of a custom event.
*/
protected function customEvent ( $event, $customDisplay, $customDisplayFields, $index ) {
$oe = ($index&1) ? 'odd' : 'even';
$content .= "<div class=\"civievent-widget-event civievent-widget-event-$oe civievent-widget-event-$index\">";
foreach ( $customDisplay as $name => $fieldAttrs ) {
if ( empty( $event[ $name ] ) ) {
if ( array_key_exists( $name, $customDisplayFields ) ) {
$fieldVal = self::getCustomDisplayField( $name, $event );
} else {
continue;
}
} else {
$fieldVal = $event[ $name ];
}
$rowField = empty( $fieldAttrs['prefix'] ) ? '' : wp_filter_kses( $fieldAttrs['prefix'] );
if ( ! empty( $fieldAttrs['title'] ) ) {
$rowField .= empty( $fieldAttrs['wrapper'] ) ? "{$fields[ $name ]}: " : "<span class=\"civievent-widget-custom-label\">{$fields[ $name ]}: </span>";
}
$rowField .= empty( $fieldAttrs['wrapper'] ) ? $fieldVal : "<span class=\"civievent-widget-custom-value\">$fieldVal</span>";
$rowField .= empty( $fieldAttrs['suffix'] ) ? '' : wp_filter_kses( $fieldAttrs['suffix'] );

$rowClass = sanitize_html_class( "civievent-widget-custom-display-$name" );
$content .= empty( $fieldAttrs['wrapper'] ) ? "$rowField\n" : "<span class=\"$rowClass\">$rowField</span>\n";
}
$content .= '</div>';
return $content;
}

/**
* Generate HTML representation of a standard event
*
* @param array $event
* The event details.
* @param array $instance
* The widget instance.
* @param integer $index
* The index of this event.
* @return string
* An HTML representation of a standard event.
*/
protected function standardEvent( $event, $instance, $index ) {
$oe = ($index&1) ? 'odd' : 'even';
$url = CRM_Utils_System::url( 'civicrm/event/info', 'reset=1&id=' . $event['id'], true, null, false );
$title = CRM_Utils_Array::value( 'title', $event );
$summary = CRM_Utils_Array::value( 'summary', $event, '' );
$content = "<div class=\"civievent-widget-event civievent-widget-event-$oe civievent-widget-event-$index\">";
$content .= $this->dateFix( $event, 'civievent-widget-event' );
if ( $title ) {
$content .= ' <span class="civievent-widget-event-title">';
$content .= self::locFix( $event, $event['id'], $instance, 'civievent-widget-event' );
$content .= '<span class="civievent-widget-infolink">';
$content .= ($url) ? "<a href=\"$url\">$title</a>" : $title;
$content .= '</span>';

$content .= self::regFix( $event, $event['id'], 'civievent-widget' );

if ( $instance['summary'] ) {
$content .= "<span class=\"civievent-widget-event-summary\">$summary</span>";
}
$content .= '</span>';
}

$content .= "</div>";
return $content;
}

/**
* Format a select list of event types
*
Expand Down