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

gpns-add-other-fields-to-date-schedule.php: Added other fields to the GP Notification Scheduler. #618

Merged
merged 2 commits into from
Jan 10, 2025

Conversation

saifsultanc
Copy link
Contributor

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/2115235612/42388?folderId=7098280

Summary

Add a field other than the Date field to the Notification Scheduler. In this instance, a "Single Line Text Field" was used.

@github-actions
Copy link

github-actions bot commented Feb 10, 2023

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against aecfdc3

Copy link
Contributor

@claygriffiths claygriffiths left a comment

Choose a reason for hiding this comment

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

LGTM! Just be sure to fix up the PHPCS issues.

I'd also go with something like this for the commit message:

gpns-add-other-fields-to-date-schedule.php: Added snippet to add a specific field ID to be treated as a date field for scheduling.

Comment on lines +15 to +16
// add the target field to fields array
array_push( $fields, $field );
Copy link
Contributor

Choose a reason for hiding this comment

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

Would $fields[] = $field; work here?

Copy link
Contributor

@scyt scyt left a comment

Choose a reason for hiding this comment

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

Approved with Clay's requested changes.

@saifsultanc saifsultanc force-pushed the saif/add/42388-add-other-fields-to-gpns branch from 3d9d0b8 to 79892bf Compare January 10, 2025 11:49
@saifsultanc saifsultanc changed the title gpns-add-other-fields-to-date-schedule.php: Add other fields to the GP Notification Scheduler. gpns-add-other-fields-to-date-schedule.php: Added other fields to the GP Notification Scheduler. Jan 10, 2025
@saifsultanc saifsultanc merged commit b119d2d into master Jan 10, 2025
3 checks passed
@saifsultanc saifsultanc deleted the saif/add/42388-add-other-fields-to-gpns branch January 10, 2025 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants