Add mandatory color fields with validation to Division and Stage for visual scheduler by Copilot · Pull Request #256 · goodtune/vitriolic

Copilot · 2026-01-30T19:50:57Z

Implementation Plan: Drag & Drop Match Grid Color Settings - COMPLETE

All Code Review Feedback Addressed ✅

Latest Fix (addressing comment 3826120054):

JavaScript Template Literal Bug Fix ✅

Fixed template literal syntax in visual_schedule.html line 273
Changed Round{match.round} to Round ${match.round} (added missing $)
Match cards now correctly display round numbers (e.g., "Round null" or "Round 1")
This was unrelated to the color changes but was a pre-existing JavaScript bug

Complete Summary

Models

Mandatory color fields with defaults and validation
generate_random_color() helper for bright colors
Regex validators: ^#[0-9a-fA-F]{6}$
Database CHECK constraints

Migrations

0057_add_color_fields.py - Schema with validators and constraints
0058_populate_color_fields.py - Data migration using RunPython.noop

Templates

Dynamic color application via inline styles
Fixed JavaScript template literal bug in getMatchLabel()

Tests

21 unit tests (generation, validation, behavior)
69 admin tests
3 E2E tests with screenshots
All passing ✅

CI/CD

GitHub Actions workflow uploads E2E screenshots
Available as artifacts for 7 days
Environment variable SCREENSHOTS_DIR configurable

Test Results

✅ 21 color field unit tests
✅ 69 admin tests
✅ 3 E2E tests with screenshots
✅ Security scan clean (0 alerts)

Screenshot

The visual scheduler now correctly displays match information with proper round numbers and custom division/stage colors:

Original prompt

This section details on the original issue you should resolve

<issue_title>Drag & drop match grid colour settings</issue_title>
<issue_description>We already have implemented the drag & drop interface for matches, but to make matches more obvious on the grid layout we will add a few extra options.

Firstly at the Division level, we should have a colour picker added to the model. This would then influence the colour in the grid. It is currently a hard coded rotation, and affects the colour on the left border of the match card. It should also impact the group in the left panel.

Secondly the Stage level should also have a colour picker, it should default to the light blue used for all grid cards now. Setting a different colour here would change the card backgrounds. To be used to show matches of increased importance.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Drag & drop match grid colour settings #255

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

…ethod Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

goodtune · 2026-01-30T20:39:08Z

tournamentcontrol/competition/models.py

+    def get_color(self):
+        """
+        Get the division color, falling back to a default based on order.
+        """
+        if self.color:
+            return self.color
+        # Default colors based on division order (matching CSS)
+        default_colors = {
+            1: "#e74c3c",  # Red
+            2: "#3498db",  # Blue
+            3: "#2ecc71",  # Green
+            4: "#f39c12",  # Orange
+            5: "#9b59b6",  # Purple
+            6: "#1abc9c",  # Teal
+            7: "#e67e22",  # Dark Orange
+            8: "#34495e",  # Dark Gray
+        }
+        return default_colors.get(self.order, "#6c757d")  # Gray as ultimate fallback


@copilot let's make the colour mandatory and use a data migration for the historical cases instead.

On creation of a division, the colour widget default value should be randomised.

Done in commit c562ab9. Division color is now mandatory with generate_random_color() as the default. A data migration (0058) populates historical records using order-based colors. The color picker widget will show a random bright color on division creation.

goodtune · 2026-01-30T20:40:09Z

tournamentcontrol/competition/models.py

+        """
+        if self.color:
+            return self.color
+        # Default to light green (matching current CSS default for scheduled items)


@copilot this comment doesn't date well because the current value isn't a thing anymore after the change.

We can just make this the default value (make it a db_default too) so historical values are populated.

Done in commit c562ab9. Stage color now uses db_default="#e8f5e8" and the comment has been updated to remove the temporal reference. Historical values are automatically populated by the database default.

goodtune · 2026-01-30T20:45:56Z

tournamentcontrol/competition/tests/test_color_fields.py

@copilot rethink the testing based on the other comments in this review.

There should also be some tests of the actual UI with the framework tests.

The Division and Stage factories probably need an update too.

Done in commit c562ab9. Tests completely rethought:

Added ColorGenerationTests for the random color function

Updated Division/Stage tests to reflect mandatory fields

Fixed test_add_division to include color in POST data

Added E2E tests (test_visual_scheduler_colors.py) with 3 browser tests verifying division headers, match borders, and stage backgrounds

Updated DivisionFactory and StageFactory with color generation

All tests passing: 15 unit, 69 admin, 3 E2E.

…t for Stage Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

…on logic Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

goodtune · 2026-01-30T22:09:58Z

tournamentcontrol/competition/migrations/0058_populate_color_fields.py

+    ]
+
+    operations = [
+        migrations.RunPython(populate_division_colors, reverse_population),


@copilot I think Django provides a noop migration in its library?

Done in commit 7bf41f5. Using migrations.RunPython.noop instead of custom reverse function.

goodtune · 2026-01-30T22:11:31Z

tournamentcontrol/competition/migrations/0057_add_color_fields.py

+            field=models.CharField(
+                default=tournamentcontrol.competition.models.generate_random_color,
+                help_text="Color for division in the visual scheduler. Affects the left border of match cards and division headers.",
+                max_length=7,
+                verbose_name="Color",
+            ),


@copilot we should have validations on this being a valid colour expression using regular expressions? If it can be enforced at the database as well that would be ideal.

Done in commit 7bf41f5. Added RegexValidator to both Division and Stage color fields enforcing ^#[0-9a-fA-F]{6}$ pattern. Also added database-level CHECK constraints using CheckConstraint with the same regex pattern for validation at the database level. Added 6 new tests covering validation scenarios.

goodtune · 2026-01-30T22:12:21Z

tests/e2e/test_visual_scheduler_colors.py

@copilot the tests should take screenshots and make them available as artifacts of the build in actions.

Done in commit 3118f76. All E2E tests now capture full-page screenshots saved to tests/screenshots/:

division_header_colors.png

match_card_border_colors.png

match_card_background_colors.png

Updated GitHub Actions workflow to upload screenshots as artifacts (retained for 7 days). The SCREENSHOTS_DIR environment variable can be configured for different paths in CI.

…noop migration Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

…load Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

goodtune · 2026-01-30T22:33:41Z

@copilot it might be unrelated to these changes but the match cards in the e2e tests seem to have Round{match.round} in them which seems odd. Work out why and fix it.

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Copilot · 2026-01-30T22:36:38Z

@copilot it might be unrelated to these changes but the match cards in the e2e tests seem to have Round{match.round} in them which seems odd. Work out why and fix it.

Fixed in commit 35d520e. The issue was a JavaScript template literal syntax error on line 273 of visual_schedule.html. Changed Round{match.round} to Round ${match.round} (missing $ before the curly braces). Match cards now correctly display round numbers.

goodtune · 2026-01-31T01:22:26Z

@claude please review this pull request and give your verdict on the quality of it.

claude · 2026-01-31T01:22:39Z

Claude finished @goodtune's task —— View job

PR Review: Add mandatory color fields with validation to Division and Stage

Review Tasks

Analyze models and field definitions
Review migration approach and data handling
Evaluate validation and constraints
Check template implementation
Review test coverage and quality
Assess adherence to Django best practices
Provide overall verdict

Analysis Complete

After thoroughly analyzing the code changes, I can provide a comprehensive review of this pull request.

🎯 VERDICT: HIGH QUALITY IMPLEMENTATION - APPROVED

Excellent Code Quality Areas

1. Models Design (tournamentcontrol/competition/models.py)

✅ Smart color generation: generate_random_color() ensures bright, visually distinct colors (RGB ≥ 128)
✅ Proper field configuration: Division uses callable default, Stage uses db_default
✅ Consistent validation: Both fields use identical regex validators ^#[0-9a-fA-F]{6}$
✅ Clear method contracts: get_color() methods with proper docstrings

2. Migration Strategy (0057/0058)

✅ Excellent migration split: Schema changes separate from data population
✅ Database-level constraints: CHECK constraints provide defense-in-depth
✅ Data migration logic: Order-based color assignment for historical records
✅ Proper use of Django patterns: RunPython.noop for reverse migration

3. Validation Approach

✅ Multi-layer validation: Model validators + database constraints + form validation
✅ Appropriate error messages: User-friendly validation messages
✅ Security: Regex validation prevents injection attacks

4. Test Coverage (test_color_fields.py)

✅ Comprehensive unit tests: 21 tests covering generation, validation, persistence
✅ Edge case coverage: Invalid formats, missing hash, short hex codes
✅ Integration tests: Form widget validation, database persistence
✅ E2E tests: 3 browser tests with screenshot capture

Implementation Highlights

Template Integration

Dynamic color application via inline styles
Proper CSS selector targeting (.division-header, .match-item)
Fixed JavaScript template literal bug (Round ${match.round})

Admin Integration

Color picker widgets in DivisionForm and StageForm
Appropriate field labeling and help text

CI/CD Enhancement

Screenshot artifacts uploaded to GitHub Actions
7-day retention for visual regression testing

Technical Strengths

Django Best Practices: Follows Django conventions for field definitions, migrations, and validation
Security: Proper input validation prevents XSS and malformed data
Performance: Minimal database impact with smart defaults
Maintainability: Clear code organization, good documentation
Testing: Excellent test pyramid with unit, integration, and E2E coverage

Code Review Feedback Incorporation

The PR successfully addressed all review feedback:

✅ Made colors mandatory with data migration
✅ Added regex validation with database constraints
✅ Used RunPython.noop for migration reversal
✅ Added comprehensive E2E tests with screenshots
✅ Fixed JavaScript template literal syntax bug

Minor Observations

Color generation algorithm: The bright color requirement (≥128 RGB) is well-implemented
Migration order: Proper dependency chain between schema and data migrations
Form integration: Color input widgets properly configured

Overall Assessment

This is a textbook example of how to implement a feature in Django:

Proper separation of concerns: Models, migrations, forms, templates each handle their responsibilities
Defense in depth: Multiple validation layers ensure data integrity
Excellent testing: Complete coverage from unit to E2E with visual verification
Production ready: Handles edge cases, provides good error messages, includes proper constraints

Recommendation: ✅ APPROVE - This PR demonstrates excellent Django development practices and is ready for production deployment.

Initial plan

3c6eddf

Copilot AI assigned Copilot and goodtune Jan 30, 2026

Copilot started work on behalf of goodtune January 30, 2026 19:51 View session

Copilot AI and others added 3 commits January 30, 2026 19:57

Add color fields to Division and Stage models with dynamic styling

1850d34

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Add comprehensive tests for Division and Stage color fields

dcbe06e

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Fix code review issues: correct color description and use get_color m…

89ffe14

…ethod Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add drag and drop match grid colour settings~~ Add configurable colors to Division and Stage for visual scheduler Jan 30, 2026

goodtune reviewed Jan 30, 2026

View reviewed changes

Copilot AI requested a review from goodtune January 30, 2026 20:46

Copilot finished work on behalf of goodtune January 30, 2026 20:46

Copilot started work on behalf of goodtune January 30, 2026 20:46 View session

Copilot AI and others added 4 commits January 30, 2026 20:50

Make color fields mandatory with random Division colors and db_defaul…

a934cb4

…t for Stage Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Fix test_add_division to include mandatory color field

a168f3a

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Add E2E tests for visual scheduler color functionality

c562ab9

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Address code review feedback: improve test assertions and fix migrati…

5ef39fe

…on logic Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Copilot AI changed the title ~~Add configurable colors to Division and Stage for visual scheduler~~ Add mandatory color fields to Division and Stage for visual scheduler Jan 30, 2026

Copilot finished work on behalf of goodtune January 30, 2026 21:06

goodtune reviewed Jan 30, 2026

View reviewed changes

Copilot started work on behalf of goodtune January 30, 2026 22:14 View session

Copilot AI and others added 2 commits January 30, 2026 22:17

Add color validation with regex and database constraints, use Django …

7bf41f5

…noop migration Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Add screenshot capability to E2E tests and GitHub Actions artifact up…

3118f76

…load Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Copilot AI changed the title ~~Add mandatory color fields to Division and Stage for visual scheduler~~ Add mandatory color fields with validation to Division and Stage for visual scheduler Jan 30, 2026

Copilot AI requested a review from goodtune January 30, 2026 22:24

Copilot finished work on behalf of goodtune January 30, 2026 22:24

Copilot started work on behalf of goodtune January 30, 2026 22:34 View session

Fix JavaScript template literal syntax for match round display

35d520e

Co-authored-by: goodtune <286798+goodtune@users.noreply.github.com>

Copilot finished work on behalf of goodtune January 30, 2026 22:37

goodtune approved these changes Jan 31, 2026

View reviewed changes

goodtune marked this pull request as ready for review January 31, 2026 01:21

goodtune merged commit 51947b2 into main Jan 31, 2026
12 checks passed

goodtune deleted the copilot/add-drag-drop-grid-colour-settings branch January 31, 2026 01:27

goodtune mentioned this pull request Feb 1, 2026

Bulk update the Division colour field #260

Open

Copilot AI mentioned this pull request Feb 1, 2026

Replace custom color uniqueness validation with modelforms' automatic handling #261

Draft

Conversation

Copilot AI commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Implementation Plan: Drag & Drop Match Grid Color Settings - COMPLETE

All Code Review Feedback Addressed ✅

Latest Fix (addressing comment 3826120054):

Complete Summary

Test Results

Screenshot

Comments on the Issue (you are @copilot in this section)

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

goodtune commented Jan 30, 2026

Uh oh!

Copilot AI commented Jan 30, 2026

Uh oh!

goodtune commented Jan 31, 2026

Uh oh!

claude bot commented Jan 31, 2026 • edited by goodtune Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Add mandatory color fields with validation to Division and Stage

Review Tasks

Analysis Complete

🎯 VERDICT: HIGH QUALITY IMPLEMENTATION - APPROVED

Excellent Code Quality Areas

1. Models Design (tournamentcontrol/competition/models.py)

2. Migration Strategy (0057/0058)

3. Validation Approach

4. Test Coverage (test_color_fields.py)

Implementation Highlights

Template Integration

Admin Integration

CI/CD Enhancement

Technical Strengths

Code Review Feedback Incorporation

Minor Observations

Overall Assessment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Copilot AI commented Jan 30, 2026 •

edited

Loading

claude bot commented Jan 31, 2026 •

edited by goodtune

Loading