Skip to content

Conversation

@royendo
Copy link
Contributor

@royendo royendo commented Jan 16, 2026

IsModel, isIncremental, isErrored, isPartition
Screenshot 2026-01-16 at 11 49 18

IsModel, isIncremental, isNotErrored, isPartition
Screenshot 2026-01-16 at 11 54 03

isModel, isNotIncremental, isErrored, isNotPartitioned
Screenshot 2026-01-16 at 11 49 25

isModel, isNotIncremental, isNotErrored, isNotPartitioned
Screenshot 2026-01-16 at 11 49 29

Also, removed unnecessary Incremental refresh for non-incremental models

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@royendo royendo requested a review from ericpgreen2 January 16, 2026 16:54
@royendo
Copy link
Contributor Author

royendo commented Jan 20, 2026

Code Review

Professionalism & Code Quality: ⭐⭐⭐⭐⭐ (Excellent)

Strengths:

  • Clean addition of "errored-partitions" refresh type
  • Good conditional rendering - only shows option when applicable
  • Proper type safety with union type extension

Issues to Address:

  • Minor - The dialog description could be more specific about what "errored partitions" means for users unfamiliar with the concept.

E2E Testing: ⚠️ Missing

No E2E tests for errored partition refresh.

Recommendations:

  • Add E2E test that verifies the "Refresh Errored Partitions" menu item appears only for incremental models with errors
  • Test the refresh trigger functionality

General Comments:

  • This is a well-scoped, focused PR
  • Good decision to reuse the existing refresh dialog infrastructure
  • The getDialogTitle helper function is clean

royendo and others added 10 commits January 20, 2026 12:00
- Change getByRole("row") to locator(".row") since VirtualizedTable
  uses div elements with class="row" instead of semantic table rows
- Use specific model/source names instead of generic type labels
- Increase timeout to 60s for data loading
- Add documentation comments to test model YAML files

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@royendo royendo mentioned this pull request Jan 27, 2026
8 tasks
@royendo
Copy link
Contributor Author

royendo commented Jan 27, 2026

Closing this PR in favor of Model Overview page.

@royendo royendo closed this Jan 27, 2026
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