core icon indicating copy to clipboard operation
core copied to clipboard

Page API returns expired content if publishDate is in the future

Open fmontes opened this issue 9 months ago • 5 comments

Problem Statement: There is a customer whose publishDate is in the future, with content that has an expiration date before it. This means the content should not show, yet it is still appearing in the response.

Steps to Reproduce: I think this is happening with this specific customer to refer to this Slack for the data: https://dotcms.slack.com/archives/CSHTYUR7H/p1748029281346769

Acceptance Criteria:

  • Content with an expiration date before its publishDate must NOT be returned by the Page API response.
  • The API correctly honors the expiration context regardless of future publishDate values.

dotCMS Version: See the env here: https://dotcms.slack.com/archives/CSHTYUR7H/p1748029281346769

Proposed Objective: Customer Success

Proposed Priority: Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.:

fmontes avatar May 28 '25 18:05 fmontes

Hi @fmontes, I'd like to help with this issue.

After looking at the codebase, I found the probable root cause:

Current Issue

The Page API doesn't validate date logic when rendering content in LIVE mode. Specifically, content with both future publishDate AND expireDate < publishDate is incorrectly being returned because:

  1. The content has live:true status (was published previously)
  2. PageRenderUtil loads content using ContentletAPI.findContentletByIdentifier() without date validation
  3. Only Time Machine queries have proper date filtering - regular LIVE mode queries don't

Root Cause Analysis

  • No validation exists to prevent expireDate < publishDate during content creation
  • Page API filtering: Content with only future publishDate is correctly filtered out (no live:true status), but the date contradiction case isn't handled
  • Time Machine has proper filtering with notExpired clauses, but regular queries don't

Proposed Fix

Add date validation in PageRenderUtil when loading container content in LIVE mode:

// Skip content with invalid date configuration in LIVE mode
if (mode.showLive && hasInvalidDateLogic(contentlet)) {
    continue; // Don't render this content
}

private boolean hasInvalidDateLogic(Contentlet contentlet) {
    Date publishDate = getPublishDate(contentlet);
    Date expireDate = getExpireDate(contentlet);
    return publishDate != null && expireDate != null && expireDate.before(publishDate);
}

Why PageRenderUtil vs ContentletAPI?

  • PageRenderUtil: fix for public-facing content, preserves admin ability to see/fix problematic content
  • ContentletAPI: Would be too broad, could break admin interfaces that may need to actually see the expired contentlet

Questions

  1. Should we also add validation to prevent expireDate < publishDate during content save?
  2. Any preference on how to handle existing invalid content (I think we should hide)?
  3. Should this apply to GraphQL Page API as well as REST?

I can start implementing this fix if the approach looks good to you.

hassanazam avatar May 29 '25 22:05 hassanazam

@hassanazam thanks for the help. PR is more than welcome.

fmontes avatar May 30 '25 00:05 fmontes

https://github.com/dotCMS/core/pull/32284 who is the best person to ask for review of this PR?

hassanazam avatar Jun 04 '25 09:06 hassanazam

@hassanazam Thank you very much, I just assigned two reviwers.

fmontes avatar Jun 06 '25 20:06 fmontes

PRs:

  • https://github.com/dotCMS/core/pull/32406

github-actions[bot] avatar Jun 12 '25 03:06 github-actions[bot]

QA Failed

Tested in: 37c9434

The functionality to hide contentlets after their expiration date is working correctly.

However, during testing, I found another issue:

If a contentlet has a published version (e.g., today, June 26) and also a future version (e.g., July 1st), the contentlet is only rendered on June 26 and starting again on July 1st. It is not being displayed on the days in between (June 27, 28, 29, and 30 in this example).

More details are explained in the attached video.

This is likely unrelated to the current ticket, but for now, I’m marking it as QA Failed to track it until the team is fully back

https://github.com/user-attachments/assets/1ead57aa-5764-4c2e-a80f-a0213b3e5de7

KevinDavilaDotCMS avatar Jun 26 '25 19:06 KevinDavilaDotCMS

There is a limitation in the system design; let's remember that the sysPublishDate and sysExpireDate are defined at the identifier level. Therefore, there can only be one publication date and expiration date per piece of content. This is a known limitation of the system.

fabrizzio-dotCMS avatar Jun 27 '25 05:06 fabrizzio-dotCMS

I don't think the issue mentioned by @KevinDavilaDotCMS was introduced by these changes, I think it has always worked this way. For me, this is out of the scope of the initial issue, and as the limitation @fabrizzio-dotCMS explained, fixing it would be a good enhancement. I would create a separate issue for this specific case.

nollymar avatar Jun 27 '25 22:06 nollymar

QA Passed

Later read the comments, i let's mark this ticket as QA Passed

Thank you very much @fabrizzio-dotCMS and @nollymar for your comments 😄

KevinDavilaDotCMS avatar Jun 27 '25 22:06 KevinDavilaDotCMS