-
Notifications
You must be signed in to change notification settings - Fork 75
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
Add attendee survey to wordcamps. #997
Add attendee survey to wordcamps. #997
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far 👍🏻
I didn't read most of the code yet, though, since you mentioned wanting to discuss some things before going further. I mostly just responded to the things in the PR description.
LMK if there's anything else you'd like me to review at this point.
public_html/wp-content/plugins/wordcamp-attendee-survey/wordcamp-attendee-survey.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/wordcamp-attendee-survey/includes/cron.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/wordcamp-attendee-survey/includes/email.php
Outdated
Show resolved
Hide resolved
$email .= "Thank you in advance, we really appreciate your time!\r\n\r\n"; | ||
$email .= "Best regards,\r\n"; | ||
$email .= "Organising Team,\r\n"; | ||
$email .= esc_html( $wordcamp_name ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious what is a better way to do this. It also isn't localized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like below would work? (exclude shortcodes and HTML tags from the i18n functions)
$email = "Hi [first_name] [last_name],\r\n\r\n";
$email .= sprintf(
/* translators: %s: wordcamp name. */
__( "%s is over, thank you to everyone who joined us! \r\n\r\n", 'wordcamporg' ),
esc_html( $wordcamp_name )
);
$email .= __( "As a community-led event, feedback is really important to us. It helps us improve our events and to keep providing high quality content.\r\n\r\n", 'wordcamporg' );
$email .= sprintf( __(
/* translators: Please leave %1$s and %2$s untranslated. They represent the opening and closing anchor tags, respectively. */
'Please take a moment to answer our %1$spost-event survey%2$s and help us to do amazing WordPress events!\r\n', 'wordcamporg' ),
'<a href="' . esc_url( $survey_page_url ) . '">', '</a>'
);
$email .= __( "(If you can't open the link, copy and paste the following URL)\r\n", 'wordcamporg' );
$email .= $survey_page_url . "\r\n\r\n";
$email .= sprintf(
/* translators: %s: closing date. */
__( "Please complete the survey by %s.\r\n\r\n", 'wordcamporg' ),
$closing_date );
$email .= __( "Please also note that all responses will be kept confidential, and we will not share your personal information with any third parties.\r\n\r\nThank you in advance, we really appreciate your time!\r\n\r\nBest regards,\r\nOrganising Team,\r\n", 'wordcamporg' );
$email .= esc_html( $wordcamp_name );
public_html/wp-content/plugins/wordcamp-attendee-survey/tests/test-wordcamp-attendee-survey.php
Outdated
Show resolved
Hide resolved
* @return bool | ||
*/ | ||
function can_load() { | ||
$skip_feature = wcorg_skip_feature( get_feature_id() ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be good to open an issue in the milestone that reminds us to remove this after iterating, otherwise it'd be easy for it to get overlooked and no one ends up using the plugin.
public_html/wp-content/plugins/wordcamp-attendee-survey/tests/test-wordcamp-attendee-survey.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/wordcamp-attendee-survey/wordcamp-attendee-survey.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/wordcamp-attendee-survey/includes/cron.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have gone through all the test cases I could think of, and everything appears to work as described 👍 Will delve deeper into the code.
public_html/wp-content/plugins/camptix-attendee-survey/includes/page.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/camptix-attendee-survey/camptix-attendee-survey.php
Outdated
Show resolved
Hide resolved
public_html/wp-content/plugins/camptix-attendee-survey/includes/cron.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍! Thanks for handling this issue, it's great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍🏻
* | ||
* @return array | ||
*/ | ||
function get_site_ids_without_skip_flag() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we've needed this here and in the Speaker Feedback plugin, I think it makes sense to extract it into a generic function in mu-plugins/wp-cli-commands/miscellaneous.php
, next to set_skip_feature_flag()
.
That way any plugin can use it, rather than duplicating it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wondered about just removing it since we're limiting it to events and this survey will probably apply across the board.
I think I'll remove it. Yep.
// TODO: This should be tested elsewhere. | ||
if ( 'events.wordpress.test' !== $blog_details->domain ) { | ||
continue; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this'll need to be updated to work on production.
You could use get_top_level_domain()
in place of a hardcoded .test
, but I think it'd be a bit more clear to check if $site->blog_id === EVENTS_NETWORK_ID
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. Makes sense.
Fixes #929
This PR is an attempt at implementing the user flow described in this comment.
How does this work programmatically?
When the plugin is activated:
tix_email
in draft statuscentral.wordpress.org
daily
cron job that looks if camp ended 2 days ago.When the cron job runs:
tix_email
tix_email
to the queue (set topending
)tix_scheduled_every_ten_minutes
Screenshots
How to test
wp-admin/edit.php?post_type=tix_ticket
wp-admin/edit.php?post_type=tix_ticket&page=camptix_options&tix_section=payment
sk_test_4eC39HqLyjWDarjtT1zdp7dc
pk_test_TYooMQauvdEDq54NiTphI7jx
Attendee Survey
in `draft modewp cron event list --url=https://events.wordpress.test/{city}/{year}/{type}
wc_attendee_survey_email
wp post update {post_id} --post_status=wcpt-closed --url=https://central.wordcamp.test/
wp cron event run wc_attendee_survey_email --url=https://events.wordpress.test/{city}/{year}/{type}
wp cron event list --url=https://events.wordpress.test/{city}/{year}/{type}
wc_attendee_disable_survey
.[[first_name], tell us what you thought: Post-event survey]
http://localhost:1080/
wp cron event run wc_attendee_disable_survey --url=https://events.wordpress.test/{city}/{year}/{type}
https://central.wordcamp.test/
click on "Attendee Survey"