Corrected email frequency and added tests.

server/src/main/java/com/objectcomputing/checkins/services/pulse/PulseServicesImpl.java

@@ -40,6 +40,8 @@ private class Frequency {
   private final SettingsServices settingsServices;
   private final Map<String, Boolean> sent = new HashMap<String, Boolean>();
 
+  private final DayOfWeek emailDay = DayOfWeek.MONDAY;
+
   private String setting = "bi-weekly";
   private final Map<String, Frequency> frequency = Map.of(
     "weekly", new Frequency(1, ChronoUnit.WEEKS),
@@ -59,11 +61,12 @@ public PulseServicesImpl(
   }
 
   public void sendPendingEmail(LocalDate check) {
-    if (check.getDayOfWeek() == DayOfWeek.MONDAY) {
+    if (check.getDayOfWeek() == emailDay) {
       LOG.info("Checking for pending Pulse email");
-      final LocalDate now = LocalDate.now();
-      LocalDate start = now.with(
-                          TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY));
+      // Start from the first of the year and move forward to ensure that we
+      // are sending email during the correct week.
+      LocalDate start = check.with(TemporalAdjusters.firstDayOfYear())
+                             .with(TemporalAdjusters.firstInMonth(emailDay));
 
       try {
         Setting freq = settingsServices.findByName("PULSE_EMAIL_FREQUENCY");
@@ -96,7 +99,15 @@ public void sendPendingEmail(LocalDate check) {
           break;
         }
         start = start.plus(freq.getCount(), freq.getUnits());
-      } while(start.getMonth() == now.getMonth());
+
+        // Apply firstInMonth(emailDay) to support adding one month to the start
+        // date.  When adding weeks, it remains on the original day.  But, when
+        // adding months, it can move away from the first of the month and we
+        // need the day specified by emailDay.
+        if (freq.getUnits() == ChronoUnit.MONTHS) {
+          start = start.with(TemporalAdjusters.firstInMonth(emailDay));
+        }
+      } while(start.isBefore(check) || start.isEqual(check));
     }
   }
 

server/src/test/java/com/objectcomputing/checkins/services/pulse/PulseServicesTest.java

@@ -0,0 +1,157 @@
+package com.objectcomputing.checkins.services.pulse;
+
+import com.objectcomputing.checkins.configuration.CheckInsConfiguration;
+import com.objectcomputing.checkins.notifications.email.MailJetFactory;
+import com.objectcomputing.checkins.services.MailJetFactoryReplacement;
+import com.objectcomputing.checkins.services.TestContainersSuite;
+import com.objectcomputing.checkins.services.fixture.TeamFixture;
+import com.objectcomputing.checkins.services.fixture.RoleFixture;
+import com.objectcomputing.checkins.services.memberprofile.MemberProfileServices;
+import com.objectcomputing.checkins.services.settings.SettingsServices;
+import com.objectcomputing.checkins.services.settings.Setting;
+import com.objectcomputing.checkins.services.memberprofile.MemberProfile;
+
+import io.micronaut.core.util.StringUtils;
+import io.micronaut.context.annotation.Property;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Named;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.time.LocalDate;
+import java.util.List;
+import java.util.Arrays;
+import java.util.stream.Collectors;
+import java.time.DayOfWeek;
+import java.time.temporal.ChronoUnit;
+import java.time.temporal.TemporalAdjusters;
+
+import static com.objectcomputing.checkins.services.role.RoleType.Constants.ADMIN_ROLE;
+import static com.objectcomputing.checkins.services.role.RoleType.Constants.MEMBER_ROLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+@Property(name = "replace.mailjet.factory", value = StringUtils.TRUE)
+class PulseServicesTest extends TestContainersSuite implements TeamFixture, RoleFixture {
+    @Inject
+    @Named(MailJetFactory.HTML_FORMAT)
+    private MailJetFactoryReplacement.MockEmailSender emailSender;
+
+    @Inject
+    CheckInsConfiguration checkInsConfiguration;
+
+    @Inject
+    MemberProfileServices memberProfileServices;
+
+    @Inject
+    SettingsServices settingsServices;
+
+    @Inject
+    PulseServices pulseServices;
+
+    private MemberProfile member;
+    private MemberProfile other;
+    private MemberProfile admin;
+    private String recipients;
+
+    private LocalDate weeklyDate;
+    private LocalDate biWeeklyDate;
+    private LocalDate monthlyDate;
+
+    private final String pulseSettingName = "PULSE_EMAIL_FREQUENCY";
+    private final String pulseWeekly = "weekly";
+    private final String pulseBiWeekly = "bi-weekly";
+    private final String pulseMonthly = "monthly";
+
+    @BeforeEach
+    void setUp() {
+        createAndAssignRoles();
+
+        member = createADefaultMemberProfile();
+        other = createASecondDefaultMemberProfile();
+
+        admin = createAThirdDefaultMemberProfile();
+        assignAdminRole(admin);
+
+        List<String> recipientsEmail = List.of(member.getWorkEmail(),
+                                               other.getWorkEmail(),
+                                               admin.getWorkEmail());
+        recipients = String.join(",", recipientsEmail);
+        emailSender.reset();
+
+        final LocalDate start =
+          LocalDate.now()
+                   .with(TemporalAdjusters.firstDayOfYear())
+                   .with(TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY));
+
+        weeklyDate = start.plus(23, ChronoUnit.WEEKS);
+        biWeeklyDate = start.plus(42, ChronoUnit.WEEKS);
+        monthlyDate = start.plus(2, ChronoUnit.MONTHS)
+                        .with(TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY));
+    }
+
+    @Test
+    void testBiWeeklySendEmail() {
+        final Setting setting = new Setting(pulseSettingName, pulseBiWeekly);
+        settingsServices.save(setting);
+
+        pulseServices.sendPendingEmail(biWeeklyDate);
+        assertEquals(1, emailSender.events.size());
+
+        final List<String> event = emailSender.events.get(0);
+        assertEquals(6, event.size());
+        assertEquals(recipients, event.get(5));
+    }
+
+    @Test
+    void testWeeklySendEmail() {
+        final Setting setting = new Setting(pulseSettingName, pulseWeekly);
+        settingsServices.save(setting);
+
+        pulseServices.sendPendingEmail(weeklyDate);
+        assertEquals(1, emailSender.events.size());
+
+        final List<String> event = emailSender.events.get(0);
+        assertEquals(6, event.size());
+        assertEquals(recipients, event.get(5));
+    }
+
+    @Test
+    void testMonthlySendEmail() {
+        final Setting setting = new Setting(pulseSettingName, pulseMonthly);
+        settingsServices.save(setting);
+
+        pulseServices.sendPendingEmail(monthlyDate);
+        assertEquals(1, emailSender.events.size());
+
+        final List<String> event = emailSender.events.get(0);
+        assertEquals(6, event.size());
+        assertEquals(recipients, event.get(5));
+    }
+
+    @Test
+    void testDuplicateSendEmail() {
+        final Setting setting = new Setting(pulseSettingName, pulseMonthly);
+        settingsServices.save(setting);
+
+        pulseServices.sendPendingEmail(monthlyDate);
+        // This should be zero because email was already sent on this date.
+        assertEquals(0, emailSender.events.size());
+    }
+
+    @Test
+    void testNoSendEmail() {
+        final Setting setting = new Setting(pulseSettingName, pulseBiWeekly);
+        settingsServices.save(setting);
+
+        pulseServices.sendPendingEmail(weeklyDate);
+        // This should be zero because, when set to bi-weekly, email is sent on
+        // the first, third, and fifth Monday of the month.
+        assertEquals(0, emailSender.events.size());
+
+        final LocalDate nonMonday = weeklyDate.plus(1, ChronoUnit.DAYS);
+        pulseServices.sendPendingEmail(nonMonday);
+        // This should be zero because the date is not a Monday.
+        assertEquals(0, emailSender.events.size());
+    }
+}