Skip to content

Resolve PR #90 merge conflicts in homepage championship logic and tests#91

Merged
dai merged 1 commit into
codex/fix-changes-not-reflecting-after-merging-pr83-c7w2krfrom
copilot/resolve-merge-conflict-90
May 24, 2026
Merged

Resolve PR #90 merge conflicts in homepage championship logic and tests#91
dai merged 1 commit into
codex/fix-changes-not-reflecting-after-merging-pr83-c7w2krfrom
copilot/resolve-merge-conflict-90

Conversation

Copilot AI commented May 24, 2026

Copy link
Copy Markdown
Contributor

PR #90 diverged from main in app/page.tsx and app/page.test.tsx, leaving unresolved conflicts around championship day labeling and leader-link rendering. This change reconciles both lines of work so the homepage keeps the newer dynamic leader behavior while preserving stabilized current-day calculation.

  • Conflict resolution: homepage day calculation + championship rendering

    • Kept the dynamic championship table/links implementation from main (buildChampionshipLeaders, result anchor links).
    • Reintroduced and exported getCurrentDay() to preserve the branch’s stabilized day derivation logic:
      • published result max day
      • known day by resultUpdatedAt reference date
      • fallback to today.makuuchi.day
    • Switched Home to use getCurrentDay() as the single source for championship visibility/label computation.
  • Conflict resolution: test expectations

    • Merged test intent from both sides:
      • retained link-to-bout coverage for championship rikishi
      • updated heading assertion to support both label paths (十四日目終了時点 at day 14, otherwise ${day}日目) via getCurrentDay().
const expectedDay = getCurrentDay();
const expectedLabel = expectedDay === 14 ? '十四日目終了時点' : `${expectedDay}日目`;

expect(screen.getByRole('heading', { level: 2, name: `優勝争い(${expectedLabel})` })).toBeInTheDocument();
expect(screen.getByRole('heading', { level: 3, name: `幕内優勝争い(${expectedLabel})` })).toBeInTheDocument();

Copilot AI assigned Copilot and dai May 24, 2026
@dai dai marked this pull request as ready for review May 24, 2026 04:29
@dai dai merged commit cebee2a into codex/fix-changes-not-reflecting-after-merging-pr83-c7w2kr May 24, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec8df54006

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/page.tsx
Comment on lines +149 to +151
const championshipLabel = currentDay === 14
? t('home.championshipLabelDay14')
: t('home.championshipLabelDay', { day: currentDay });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Base championship label on published results day

championshipLabel is derived from currentDay, but buildChampionshipLeaders only aggregates resultDays where status === 'published'. In the common case where the next day exists as pending, getCurrentDay() can advance to that day while leader stats are still from the prior published day, so the heading shows a newer day label than the standings actually represent. Derive the label from the same published-day source used for championshipLeaders (or return that day from the builder) to keep the UI consistent.

Useful? React with 👍 / 👎.

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