Skip to content

refactor: Replace hardcoded date cycle logic#596

Merged
dricazenck merged 5 commits intoWomen-Coding-Community:mainfrom
dricazenck:refactor/db-driven-mentorship-cycle
Apr 2, 2026
Merged

refactor: Replace hardcoded date cycle logic#596
dricazenck merged 5 commits intoWomen-Coding-Community:mainfrom
dricazenck:refactor/db-driven-mentorship-cycle

Conversation

@dricazenck
Copy link
Copy Markdown
Collaborator

@dricazenck dricazenck commented Apr 2, 2026

Description

  • MentorshipService.getCurrentCycle() previously used hard-coded
    London-timezone date windows to determine the active cycle; replaced
    with cycleRepository.findOpenCycle() so cycle state is database-driven.
  • MenteeService simplified to query the open cycle directly, removing
    the backward-compatibility fallback path.
  • Added @JsonIgnoreProperties(ignoreUnknown = true) to MentorDto,
    MemberDto, and MentorFilterSection to tolerate unknown JSON fields.
  • All affected unit and integration tests updated.

Change Type

  • Bug Fix
  • Code Refactor

MentorshipService previously determined the active cycle using London-timezone
date windows hard-coded to specific months and day counts. Replace this with
a cycleRepository.findOpenCycle() call so cycle state is driven by the
database. MenteeService is simplified to use the same repository directly,
removing the backward-compatibility fallback. Add @JsonIgnoreProperties to
DTO classes to tolerate unknown fields. Tests updated throughout.
@dricazenck dricazenck requested a review from a team as a code owner April 2, 2026 20:37
@dricazenck dricazenck changed the title refactor: Replace hardcoded date cycle logic with DB-driven cycle lookup refactor: Replace hardcoded date cycle logic Apr 2, 2026
The daysCycleOpen property and DEFAULT_DAYS constant are no longer used
now that cycle state is DB-driven. Remove the field from MentorshipConfig,
drop the property from application.yml, and delete the two daysCycleOpen
tests from MentorshipConfigTest. Clean up dead MentorshipConfig and
MentorshipService mocks from MenteeServiceTest, and remove the leftover
unused daysOpen variable from MentorshipServiceTest setUp.
Copy link
Copy Markdown
Contributor

@womencodingcommunity womencodingcommunity left a comment

Choose a reason for hiding this comment

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

Solid refactor — removing the London-timezone date windows in favour of a DB-driven open cycle makes the system much more predictable and operationally controllable. The simplification to MenteeService is clean and the dead-code cleanup is thorough. Two optional quick-wins worth addressing: a redundant null ternary in toOpenCycleValue() and a copy-paste Javadoc on getOpenCycle(). The cycleYear field on MenteeRegistration being silently ignored is worth a follow-up but not a blocker.

In toOpenCycleValue(), the expression 'cycleMonth != null ? cycleMonth : null'
is equivalent to just 'cycleMonth' — remove the redundant ternary. Fix the
copy-pasted Javadoc on getOpenCycle() which incorrectly described the removed
getMentorshipCycle() parameters instead of the actual no-arg signature.
Move Jackson unknown-property tolerance from per-class @JsonIgnoreProperties
annotations to a single global DeserializationFeature configuration in
ObjectMapperConfig, reducing annotation noise and ensuring consistent
deserialization behaviour across all DTOs.
Remove unused getOpenCycle() from MentorshipService as no callers remain.
Remove the unknown-field-causes-400 controller test now that
FAIL_ON_UNKNOWN_PROPERTIES is globally disabled in ObjectMapperConfig.
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 2, 2026

@dricazenck dricazenck merged commit 4626d1e into Women-Coding-Community:main Apr 2, 2026
5 of 6 checks passed
@dricazenck dricazenck deleted the refactor/db-driven-mentorship-cycle branch April 2, 2026 21:30
@dricazenck dricazenck restored the refactor/db-driven-mentorship-cycle branch April 4, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants