BREAKING CHANGE: change EventTime enum #7899
Conversation
|
dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
Outdated
Show resolved
Hide resolved
dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
Outdated
Show resolved
Hide resolved
|
Warning Rate limit exceeded@dxrknesss has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 10 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThe pull request updates the event filtering terminology throughout the application. In the HTML template, a checkbox input and its label were modified from "Future" to "Upcoming." The repository implementation now uses a switch statement to handle event times, accommodating the new "UPCOMING" value, and the corresponding enum constant has been renamed from FUTURE to UPCOMING. Additionally, new integration tests and a PostgreSQL initializer have been added to support repository testing. Changes
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/src/main/resources/templates/core/management_events.html(1 hunks)dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java(1 hunks)dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java(1 hunks)dao/src/test/java/greencity/repository/util/PostgresInitializer.java(1 hunks)service-api/src/main/java/greencity/enums/EventTime.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- service-api/src/main/java/greencity/enums/EventTime.java
🧰 Additional context used
🪛 GitHub Actions: CI/CD GreenCity
dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
[error] 148-148: switch without "default" clause
🔇 Additional comments (2)
dao/src/test/java/greencity/repository/util/PostgresInitializer.java (1)
1-18: LGTM! Well-structured test database setup.The implementation provides a robust and reusable test database setup using testcontainers. The class is correctly marked as abstract, and the properties are properly configured for both datasource and liquibase.
core/src/main/resources/templates/core/management_events.html (1)
176-178: LGTM! Consistent terminology update.The checkbox label and value have been correctly updated from "Future" to "Upcoming", maintaining consistency with the backend changes.
dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java
Outdated
Show resolved
Hide resolved
dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dao/src/test/java/greencity/ModelUtils.java (2)
14-17: Remove unused imports.The following imports are not used in the code:
java.sql.Datejava.time.temporal.ChronoUnitjava.time.temporal.TemporalAmount-import java.sql.Date; -import java.time.temporal.ChronoUnit; -import java.time.temporal.TemporalAmount;
34-104: Reduce code duplication by extracting common builder patterns.The method contains duplicated builder patterns for creating event DTOs. Consider extracting common patterns into helper methods.
+ private static EventDto.EventDtoBuilder baseEventDtoBuilder() { + return EventDto.builder() + .type(EventType.ONLINE) + .isOpen(true) + .isSubscribed(true) + .isFavorite(true) + .isRelevant(true) + .likes(0) + .countComments(2) + .isOrganizedByFriend(false) + .eventRate(3.5) + .description("123") + .titleImage("image.png"); + } + + private static AddressDto.AddressDtoBuilder baseAddressBuilder() { + return AddressDto.builder() + .latitude(0.0) + .longitude(1.0) + .cityEn("Kyiv"); + } + public static List<EventDto> getListEventDto() { return List.of( - EventDto.builder() + baseEventDtoBuilder() .id(3L) .title("test3") .organizer(EventAuthorDto.builder().id(1L).name("Test3").build()) .creationDate(LocalDate.now().plus(Period.ofDays(12))) // ... rest of the builder chain
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dao/pom.xml(1 hunks)dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java(3 hunks)dao/src/test/java/greencity/ModelUtils.java(2 hunks)dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
- dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java
🔇 Additional comments (2)
dao/pom.xml (1)
108-114: Test Dependency for ModelMapper AddedThe new dependency for
org.modelmapper:modelmapperis correctly added with a test scope. Please ensure that the property${modelmapper.version}is defined in your project (or parent POM) to avoid any dependency resolution issues at build time.dao/src/test/java/greencity/ModelUtils.java (1)
34-104:❓ Verification inconclusive
Add test data for the UPCOMING event time.
Given that this PR introduces a breaking change by renaming the 'FUTURE' enum value to 'UPCOMING', consider adding test data that explicitly covers this scenario.
Let's verify if the EventTime enum is used in the EventDto:
🏁 Script executed:
#!/bin/bash # Description: Check if EventDto uses EventTime enum ast-grep --pattern 'class EventDto { $$$ EventTime $_ = $_ $$$ }'Length of output: 73
Test Data Update Required for UPCOMING Event Time
It appears that the test data in
dao/src/test/java/greencity/ModelUtils.javaonly creates events with an ONLINE type. Since the PR renames theFUTUREenum value toUPCOMING, please add a test case that covers this new state. Note that our automated check usingast-grepdid not reveal any usage of anEventTimefield inEventDto, so please manually verify whether the renamed enum impacts additional parts of the codebase or if additional test data is required elsewhere.
- Action Items:
- Create a test case in the relevant test data (e.g., in
ModelUtils.java) that uses the UPCOMING event time.- Manually review related tests to confirm that all scenarios relying on the renamed enum value are correctly updated.
dxrknesss
force-pushed
the
bug/6824/future-upcoming
branch
from
feb68a3 to
997457c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
dao/src/test/java/greencity/ModelUtils.java (1)
43-44: 🛠️ Refactor suggestionUse fixed dates instead of relative ones to ensure test determinism.
The current implementation uses
ZonedDateTime.now()with relative offsets, which could make tests non-deterministic.Also applies to: 77-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dao/pom.xml(1 hunks)dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java(2 hunks)dao/src/test/java/greencity/ModelUtils.java(2 hunks)dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- dao/pom.xml
- dao/src/test/java/greencity/repository/impl/EventSearchRepoImplTest.java
- dao/src/main/java/greencity/repository/impl/EventSearchRepoImpl.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
dao/src/test/java/greencity/ModelUtils.java (1)
3-19: LGTM!The imports are well-organized and all are utilized in the implementation.
dxrknesss
force-pushed
the
bug/6824/future-upcoming
branch
from
997457c to
2672cb4
Compare
|
GreenCity PR
Issue Link 📋
#6824
Changed
Summary of Changes
This change should fix the bug from issue #6824, if merged at the same time with front-end fix.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
modelmapperlibrary to facilitate testing.