Camptix: Notify: Add attended segment #288
Conversation
| 'key' => 'tix_attended', | ||
| 'value' => 1, | ||
| ); | ||
| switch ( $condition['op'] ) { |
There was a problem hiding this comment.
This block might be more readable as an if/else statement - what do you think?
There was a problem hiding this comment.
I agree an if/else statement would be more readable. I was using the same pattern I had seen for the other segments.
Here are links to those:
- https://github.com/WordPress/wordcamp.org/blob/production/public_html/wp-content/plugins/camptix/camptix.php#L3403
- https://github.com/WordPress/wordcamp.org/blob/production/public_html/wp-content/plugins/camptix/camptix.php#L3421
- https://github.com/WordPress/wordcamp.org/blob/production/public_html/wp-content/plugins/camptix/camptix.php#L3441
Would you suggest I change the others as well?
There was a problem hiding this comment.
Yeah, I saw the others– I also think those would be more readable as if/else, but no, I wouldn't touch those in this PR. It's okay if this one looks different, the idea is to iteratively make this code better, not refactor everything all at once.
| if ( empty( $condition['field'] ) || empty( $condition['op'] ) ) | ||
| continue; | ||
|
|
||
| if ( "attended" != $condition['field'] && ! isset( $condition['value'] ) ) |
There was a problem hiding this comment.
Some PHP code style nitpicks: this should use single quotes around attended, and use strict comparisons:
| if ( "attended" != $condition['field'] && ! isset( $condition['value'] ) ) | |
| if ( 'attended' !== $condition['field'] && ! isset( $condition['value'] ) ) |
| @@ -3460,6 +3470,26 @@ public function get_segment( $relation, $conditions ) { | |||
| $post_query_conditions[] = $condition; | |||
| continue; | |||
| } | |||
|
|
|||
| // Attended the event | |||
| if ( 'attended' == $condition['field'] ) { | |||
There was a problem hiding this comment.
Again, this should use a strict comparison:
| if ( 'attended' == $condition['field'] ) { | |
| if ( 'attended' === $condition['field'] ) { |
|
Hey @CdrMarks, are you still working on this issue? Let me know if you'd like to me to take it over and merge it 🙂 |
Can now segment attendees by if they attended the event.
Fixes #280.
Reattempt after feedback on #281.