Hide topics with past application deadlines and add EXPIRED state#1089
Conversation
|
Warning Review limit reached
More reviews will be available in 51 minutes and 32 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds a new EXPIRED topic state, computes it server-side from past applicationDeadline values, updates repository/service/cron logic to treat published/expired topics correctly, enforces published-only applications, and updates client UI to use topic.state (including an orange badge) instead of local deadline checks. ChangesTopic expiration state lifecycle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx (1)
133-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply CTA should be limited to OPEN topics.
Line 133 only diverts
EXPIRED;CLOSEDcurrently reaches theApplybranch.Suggested fix
- ) : 'state' in topic && topic.state === TopicState.EXPIRED ? ( + ) : 'state' in topic && topic.state !== TopicState.OPEN ? ( <Button fullWidth mt='md' component={Link} to={`/topics/${topicId}`}> More Information </Button> ) : ( <Button fullWidth mt='md' component={Link} to={`/submit-application/${topicId}`}> Apply </Button> )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx` around lines 133 - 140, The current conditional only checks for TopicState.EXPIRED and falls back to the Apply button, which allows CLOSED topics to show "Apply"; update the branch logic in TopicCard (the JSX that inspects 'topic' and 'topic.state' and renders Button with 'to={`/submit-application/${topicId}`}`) so that the Apply CTA is rendered only when topic.state === TopicState.OPEN, and render the More Information (or a non-apply variant) for any non-OPEN state (e.g., TopicState.CLOSED or TopicState.EXPIRED); locate the conditional that references TopicState.EXPIRED and replace it with an explicit check for TopicState.OPEN for the Apply branch, keeping topicId and Link/Button usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@client/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsx`:
- Line 38: The current gating boolean deadlinePassed in CollapsibleTopicElement
only treats TopicState.EXPIRED as disabled, leaving CLOSED actionable; update
the expression that defines deadlinePassed (which uses fullTopic and
TopicState.EXPIRED) to treat both states as blocking by checking fullTopic.state
=== TopicState.EXPIRED || fullTopic.state === TopicState.CLOSED so CLOSED topics
are also considered disabled.
In `@server/src/main/java/de/tum/cit/aet/thesis/repository/TopicRepository.java`:
- Around line 62-63: The countOpenTopics query currently omits the
published-state check and thus counts draft topics; update the countOpenTopics
JPQL/SQL to include the same published predicate used by your OPEN query path
(e.g., add the condition that enforces published state such as the same
t.published = true or t.state = 'PUBLISHED' clause used in OPEN), keeping the
other existing predicates (t.applicationDeadline and :researchGroupId) intact so
drafts with closedAt IS NULL are no longer counted as open.
---
Outside diff comments:
In
`@client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx`:
- Around line 133-140: The current conditional only checks for
TopicState.EXPIRED and falls back to the Apply button, which allows CLOSED
topics to show "Apply"; update the branch logic in TopicCard (the JSX that
inspects 'topic' and 'topic.state' and renders Button with
'to={`/submit-application/${topicId}`}`) so that the Apply CTA is rendered only
when topic.state === TopicState.OPEN, and render the More Information (or a
non-apply variant) for any non-OPEN state (e.g., TopicState.CLOSED or
TopicState.EXPIRED); locate the conditional that references TopicState.EXPIRED
and replace it with an explicit check for TopicState.OPEN for the Apply branch,
keeping topicId and Link/Button usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e270d6b-90fc-44a0-8ce5-062ff0bb0800
📒 Files selected for processing (12)
client/src/components/TopicsTable/TopicsTable.tsxclient/src/pages/LandingPage/LandingPage.tsxclient/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsxclient/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsxclient/src/pages/TopicPage/TopicPage.tsxclient/src/requests/responses/topic.tsclient/src/utils/format.tsserver/src/main/java/de/tum/cit/aet/thesis/constants/TopicState.javaserver/src/main/java/de/tum/cit/aet/thesis/cron/AutomaticRejects.javaserver/src/main/java/de/tum/cit/aet/thesis/entity/Topic.javaserver/src/main/java/de/tum/cit/aet/thesis/repository/TopicRepository.javaserver/src/main/java/de/tum/cit/aet/thesis/service/TopicService.java
There was a problem hiding this comment.
Pull request overview
This PR introduces an EXPIRED topic state derived from applicationDeadline and updates server-side queries and client UI so that topics past their application deadline are treated as no longer open/applicable while still remaining editable/processable where needed (e.g., supervisor management and auto-reject cron).
Changes:
- Add
EXPIREDto topic state handling (server + client) and derive it dynamically fromapplicationDeadline. - Update server topic queries/counts so “open” topics exclude expired deadlines; add a “published” query for cron processing.
- Update client UI to disable/adjust “Apply” actions and show the new state in formatting/badges.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/main/java/de/tum/cit/aet/thesis/service/TopicService.java | Adds EXPIRED access handling and a “published topics” accessor for cron usage. |
| server/src/main/java/de/tum/cit/aet/thesis/repository/TopicRepository.java | Updates “OPEN” filtering to exclude expired deadlines; adds “published topics” query. |
| server/src/main/java/de/tum/cit/aet/thesis/entity/Topic.java | Derives EXPIRED state dynamically from applicationDeadline. |
| server/src/main/java/de/tum/cit/aet/thesis/cron/AutomaticRejects.java | Switches cron to iterate published topics so expired topics are still processed. |
| server/src/main/java/de/tum/cit/aet/thesis/constants/TopicState.java | Adds EXPIRED enum value. |
| client/src/utils/format.ts | Adds label formatting for EXPIRED. |
| client/src/requests/responses/topic.ts | Adds EXPIRED to client TopicState enum. |
| client/src/pages/TopicPage/TopicPage.tsx | Disables applying for expired/closed topics (and updates deadline logic usage). |
| client/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsx | Switches “deadline passed” logic to use backend topic state. |
| client/src/pages/LandingPage/LandingPage.tsx | Shows “Apply” CTA only for OPEN topics. |
| client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx | Updates topic card CTA logic for expired topics. |
| client/src/components/TopicsTable/TopicsTable.tsx | Adds orange color mapping for EXPIRED. |
Comments suppressed due to low confidence (2)
client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx:140
- The card currently falls back to an "Apply" CTA for any non-expired topic. If a CLOSED topic is ever rendered here (e.g., via filters or future reuse), it will incorrectly show an Apply link. Safer is to show Apply only for OPEN topics and fall back to "More Information" otherwise.
) : 'state' in topic && topic.state === TopicState.EXPIRED ? (
<Button fullWidth mt='md' component={Link} to={`/topics/${topicId}`}>
More Information
</Button>
) : (
<Button fullWidth mt='md' component={Link} to={`/submit-application/${topicId}`}>
Apply
</Button>
client/src/pages/TopicPage/TopicPage.tsx:71
- deadlinePassed now also covers TopicState.CLOSED, but the UI text shown to users says "Application deadline has passed." This is incorrect for closed topics (closed != expired deadline). Consider using separate flags/messages (e.g., "Topic is closed" vs "Application deadline has passed") or a generic "Applications are no longer accepted."
const deadlinePassed = topic.state === TopicState.EXPIRED || topic.state === TopicState.CLOSED
return (
<Stack gap={'2rem'}>
<Stack gap={'1rem'}>
<Title>{topic.title}</Title>
{!managementAccess && !checkIfUserIsExaminerOrSupervisor() && (
<Stack gap='xs' mr='auto'>
{deadlinePassed ? (
<Button leftSection={<NotePencil size={24} />} size='md' disabled>
Apply Now
</Button>
) : (
<Button
component={Link}
to={`/submit-application/${topic.topicId}`}
leftSection={<NotePencil size={24} />}
size='md'
>
Apply Now
</Button>
)}
{deadlinePassed && (
<Text size='sm' c='dimmed'>
Application deadline has passed.
</Text>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@bensofficial The expiration state wiring is mostly consistent, but there are still a couple of places where non-open topics leak into application/interview flows. I left the new findings inline; the existing reviewer feedback on the open-topic count and CTA behavior still looks relevant too.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@bensofficial The earlier issues are fixed, but there is one new regression in the topic selector loading state. See the inline comment; this keeps the application flow from advancing before the topic details have loaded.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
server/src/test/java/de/tum/cit/aet/thesis/mock/EntityMockFactory.java (1)
96-98: ⚡ Quick winUse a fixed fixture timestamp instead of
Instant.now()in test factory.These fields are test fixtures; time-dependent values can make boundary tests less reproducible. Prefer a fixed instant constant.
Proposed refactor
public class EntityMockFactory { + private static final Instant FIXED_INSTANT = Instant.parse("2026-01-01T00:00:00Z"); + public static Topic createTopic(String title, ResearchGroup researchGroup) { Topic topic = new Topic(); topic.setId(UUID.randomUUID()); topic.setTitle(title); topic.setRoles(new ArrayList<>()); topic.setResearchGroup(researchGroup); - topic.setCreatedAt(Instant.now()); - topic.setUpdatedAt(Instant.now()); - topic.setPublishedAt(Instant.now()); + topic.setCreatedAt(FIXED_INSTANT); + topic.setUpdatedAt(FIXED_INSTANT); + topic.setPublishedAt(FIXED_INSTANT); return topic; } }As per coding guidelines, "Prefer fixed test data over random data for reproducibility."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/src/test/java/de/tum/cit/aet/thesis/mock/EntityMockFactory.java` around lines 96 - 98, Replace the time-dependent Instant.now() calls in EntityMockFactory when building Topic test fixtures with a fixed Instant constant; add a static final Instant (e.g., FIXED_INSTANT = Instant.parse("2020-01-01T00:00:00Z")) in the EntityMockFactory class and use FIXED_INSTANT for topic.setCreatedAt(...), topic.setUpdatedAt(...), and topic.setPublishedAt(...) so tests are reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/src/test/java/de/tum/cit/aet/thesis/mock/EntityMockFactory.java`:
- Around line 96-98: Replace the time-dependent Instant.now() calls in
EntityMockFactory when building Topic test fixtures with a fixed Instant
constant; add a static final Instant (e.g., FIXED_INSTANT =
Instant.parse("2020-01-01T00:00:00Z")) in the EntityMockFactory class and use
FIXED_INSTANT for topic.setCreatedAt(...), topic.setUpdatedAt(...), and
topic.setPublishedAt(...) so tests are reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1373d8a4-7265-43e7-9b7d-11f84d9135dd
📒 Files selected for processing (8)
client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsxclient/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsxclient/src/pages/TopicPage/TopicPage.tsxserver/src/main/java/de/tum/cit/aet/thesis/cron/AutomaticRejects.javaserver/src/main/java/de/tum/cit/aet/thesis/repository/TopicRepository.javaserver/src/main/java/de/tum/cit/aet/thesis/service/ApplicationService.javaserver/src/main/java/de/tum/cit/aet/thesis/service/TopicService.javaserver/src/test/java/de/tum/cit/aet/thesis/mock/EntityMockFactory.java
🚧 Files skipped from review as they are similar to previous changes (3)
- server/src/main/java/de/tum/cit/aet/thesis/cron/AutomaticRejects.java
- client/src/pages/LandingPage/components/TopicCardGrid/TopicCard/TopicCard.tsx
- client/src/pages/ReplaceApplicationPage/components/SelectTopicStep/components/CollapsibleTopicElement.tsx
…Step/components/CollapsibleTopicElement.tsx Co-authored-by: Claudia Anthropica <claudia.aet@xcit.tum.de>
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@bensofficial All feedback addressed, nice work.
|
Verbally approved by @vikikab! |
Resolves conflicts from develop (passkey #955, hide-expired-topics #1089, dependency updates, react-router v8) against the feature-package modularization. Resolution summary: - Imports: kept the modularized `@/` (client) and feature-package (server) import style; folded in develop's newly-added imports converted to the same style. - New passkey files added by develop landed in the old flat layout and were relocated into the modularized structure with `@/` imports: - components/PasskeyRegistrationPrompt -> core/components/PasskeyRegistrationPrompt - pages/SettingsPage/components/PasskeySettings -> core/admin/pages/SettingsPage/components/PasskeySettings - utils/passkey.ts (git-relocated) -> core/utils/passkey.ts - CollapsibleTopicElement: took develop's canonical #1089 behavior (`canApply = !!fullTopic && state === OPEN`). - BaseKeycloakIntegrationTest: kept in mock/ (consistent with BaseIntegrationTest) with modularized repository imports; TestContainerImages is same-package. Verified: client build/tsc/lint/tests green; server compiles + 864/864 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
applicationDeadlineare excluded from open topic listings, counts, and interview topic selectionEXPIREDtopic state derived dynamically fromapplicationDeadlineingetTopicState()— no database column or cronjob neededOPENtopics across all views (landing page, topic detail, topic cards, application flow)findPublishedTopicsquery so it still processes applications on expired topicscountOpenTopicsandfindOpenTopicsForUserByRolesqueries (publishedAt IS NOT NULL)createApplicationrejects applications for draft topics (in addition to closed and deadline-passed)findPublishedTopicsconverted from nativePagequery to JPQLListquery, removing theInteger.MAX_VALUEpagination workaroundTest Plan
./gradlew test— all 864 tests passpnpm exec tsc --noEmit— no type errors🤖 Generated with Claude Code
Summary by CodeRabbit