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

(dev/core#365) Scheduled Reminders - Add effective end and start date, extend unit test #19973

Merged
merged 2 commits into from
Jun 10, 2021

Conversation

monishdeb
Copy link
Member

@monishdeb monishdeb commented Apr 6, 2021

Overview

Add optional effective end and start date filter while sending schedule reminders. And extend unit-test to add the filters

Comments

ping @eileenmcnaughton

@civibot
Copy link

civibot bot commented Apr 6, 2021

(Standard links)

@civibot civibot bot added the master label Apr 6, 2021
@monishdeb monishdeb force-pushed the core_365_1 branch 2 times, most recently from f235617 to d19338e Compare April 6, 2021 18:30
@eileenmcnaughton
Copy link
Contributor

@monishdeb if you add a screenshot on the UI I'll add to the dev digest

@eileenmcnaughton
Copy link
Contributor

@monishdeb the tests are still winning

Test Result (2 failures / +2)
CRM_Core_BAO_ActionScheduleTest.testContactCustomDateNoAnniversary
CRM_Core_BAO_ActionScheduleTest.testMembershipOnMultipleReminder

@eileenmcnaughton
Copy link
Contributor

@monishdeb I took a look at this & it looks like the test fails are just bleed - ie you defined this for one test

  'subject' => 'subject sched_membership_end_2week',
      'effective_start_date' => '2012-05-01 01:00:00',

but it messes with a different one

Also the date seems wrong on grad_tomorrow because it doesn't say what it seems to - this might be better

       'start_action_offset' => '1',
       'start_action_unit' => 'day',
       'subject' => 'subject sched_contact_grad_tomorrow',
-      'effective_start_date' => date('YmdHis', strtotime('-2 days')),
+      'effective_start_date' => '2013-10-15 20:00:00',
     ];

The actual code looks good so if the tests pass the code is fine IMHO

@monishdeb
Copy link
Member Author

@eileenmcnaughton I am finally able to fix all the failing unit tests. I am pretty sure now all the SR unit-test has ESD and EED set during scheduling a reminder in different use-case. Can you please have a look.

@totten totten changed the title [WIP] Add effective end and start date, extend unit test [WIP] (dev/core#365) Scheduled Reminders - Add effective end and start date, extend unit test May 27, 2021
@monishdeb monishdeb changed the title [WIP] (dev/core#365) Scheduled Reminders - Add effective end and start date, extend unit test (dev/core#365) Scheduled Reminders - Add effective end and start date, extend unit test May 28, 2021
@eileenmcnaughton eileenmcnaughton merged commit f7ecb54 into civicrm:master Jun 10, 2021
@eileenmcnaughton
Copy link
Contributor

OK - this is passing tests & makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants