Skip to content

fix: keep competition order when merging cached standings - #26

Merged
ItamarShDev merged 1 commit into
mainfrom
devin/1788467340-league-order
Sep 9, 2026
Merged

ItamarShDev merged 1 commit into
mainfrom
devin/1788467340-league-order

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #25: mergeWithPrevious now builds leaguesData by iterating requestedLeagueIds (the order of team.runningCompetitions) and picking fresh-else-previous per id, so a failed standings request no longer shuffles that competition's table to the end.

Requires npx convex deploy after merge.

Link to Devin session: https://app.devin.ai/sessions/3fc3db4656ca4cd7a03c1bf5a7182f15
Open in Devin Desktop: https://app.devin.ai/desktop/session/3fc3db4656ca4cd7a03c1bf5a7182f15?variant=devin
Requested by: @ItamarShDev

Co-Authored-By: Itamar Sharify <itamarsharify@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author
Original prompt from Itamar

@ItamarShDev/hakapit
This repo is a mess
Use swe-1-7 subagents to read the code, clean up all comments
Review the code arch and organization and suggest fixes.

For example, why we have component under a convex folder??

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hakapit Ready Ready Preview Sep 3, 2026 8:29pm UTC

@ItamarShDev
ItamarShDev marked this pull request as ready for review September 9, 2026 10:29
Copilot AI lite review requested due to automatic review settings September 9, 2026 10:29

Copilot AI 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.

🟢 Approval recommended

The change is small, localized, and correctly preserves requested league ordering while maintaining the fresh-else-previous behavior for partial failures.

Pull request overview

This PR adjusts snapshot merging in convex/football.ts so that when standings are partially served from cache (because a subset of standings fetches fail), the resulting leaguesData order matches the order of team.runningCompetitions instead of appending “kept” cached leagues at the end.

Changes:

  • Replace set/filter-based “kept leagues” merging with per-leagueId selection (fresh-first, else previous).
  • Build leaguesData by iterating requestedLeagueIds to preserve competition ordering deterministically.
File summaries
File Description
convex/football.ts Reworks mergeWithPrevious league merge logic to preserve competition order while retaining cached entries for failed standings requests.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ItamarShDev

Copy link
Copy Markdown
Owner

@devin review and commit fixes

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Reviewed: Copilot approved with zero findings, CI green, no conflicts with main (including the #27 changes to convex/football.ts) — nothing to fix. Ready to merge, then npx convex deploy.

@ItamarShDev
ItamarShDev merged commit 90423c3 into main Sep 9, 2026
3 checks passed

This branch was successfully deployed

1 active deployment
Preview 46a9caaf Deployed Sep 3, 2026 by vercel[bot]
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