🎨 Palette: Add dashboard link to success message#414
Conversation
Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
😎 Merged manually by @abhimehro - details. |
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Summary of ChangesHello @abhimehro, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience by integrating a direct link to the Control D dashboard within the success message displayed after a successful sync operation. This allows users to quickly access and confirm their changes, streamlining the post-sync workflow. The implementation also ensures accessibility by using distinct styling for the link. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
| # Verify content | ||
| # Note: The output is ANSI colored, so exact string matching might fail if color codes are interspersed | ||
| # But "View your changes" should be there | ||
| assert "View your changes" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Note: The output is ANSI colored, so exact string matching might fail if color codes are interspersed | ||
| # But "View your changes" should be there | ||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles/123456/filters" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles/123456/filters" in combined_output | ||
| # Check for color codes presence (cyan or underline) | ||
| assert "\033[96m" in combined_output or "\033[4m" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| writes = [args[0] for args, _ in mock_stdout.write.call_args_list] | ||
| combined_output = "".join(writes) | ||
|
|
||
| assert "View your changes" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| combined_output = "".join(writes) | ||
|
|
||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles" in combined_output |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles" in combined_output | ||
| assert "/123/filters" not in combined_output # Should not link to specific profile |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
There was a problem hiding this comment.
Code Review
This pull request adds a helpful link to the Control D dashboard in the success message, improving the user experience. The implementation is clean and includes good test coverage. I've added a few suggestions to improve maintainability by reducing code duplication and moving a list to a module-level constant. I also suggested a small improvement to one of the new tests to make it more robust.
| success_msgs = [ | ||
| "✨ All synced!", | ||
| "🚀 Ready for liftoff!", | ||
| "🎨 Beautifully done!", | ||
| "💎 Smooth operation!", | ||
| "🌈 Perfect harmony!", | ||
| ] |
There was a problem hiding this comment.
The success_msgs list is defined inside the print_success_message function. This means it's recreated every time the function is called. For better performance and to follow the convention of defining constants at the module level, consider moving this list outside the function, for example, as a module-level constant like _SUCCESS_MESSAGES.
| # Verify content | ||
| # Note: The output is ANSI colored, so exact string matching might fail if color codes are interspersed | ||
| # But "View your changes" should be there | ||
| assert "View your changes" in combined_output |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Note: The output is ANSI colored, so exact string matching might fail if color codes are interspersed | ||
| # But "View your changes" should be there | ||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles/123456/filters" in combined_output |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles/123456/filters" in combined_output | ||
| # Check for color codes presence (cyan or underline) | ||
| assert "\033[96m" in combined_output or "\033[4m" in combined_output |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| writes = [args[0] for args, _ in mock_stdout.write.call_args_list] | ||
| combined_output = "".join(writes) | ||
|
|
||
| assert "View your changes" in combined_output |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| combined_output = "".join(writes) | ||
|
|
||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles" in combined_output |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| assert "View your changes" in combined_output | ||
| assert "https://controld.com/dashboard/profiles" in combined_output | ||
| assert "/123/filters" not in combined_output # Should not link to specific profile |
Check notice
Code scanning / Bandit (reported by Codacy)
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
There was a problem hiding this comment.
Pull request overview
Adds a richer “success delight” output by appending a Control D dashboard URL to the post-sync success message so users can immediately navigate to verify changes.
Changes:
- Introduces
print_success_message(profile_ids)to print a random success line plus a dashboard link (single-profile deep link vs multi-profile dashboard link). - Updates the main success path to call the new helper instead of printing the success text inline.
- Adds UX tests validating the success message/link behavior for single/multiple profiles and when colors are disabled.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.py | Adds print_success_message() and wires it into the “all success” non-dry-run path. |
| tests/test_ux.py | Adds tests asserting the success output contains the expected dashboard links and respects USE_COLORS. |
| def print_success_message(profile_ids: List[str]) -> None: | ||
| """Prints a random success message and a link to the Control D dashboard.""" | ||
| if not USE_COLORS: | ||
| return | ||
|
|
||
| success_msgs = [ | ||
| "✨ All synced!", | ||
| "🚀 Ready for liftoff!", | ||
| "🎨 Beautifully done!", | ||
| "💎 Smooth operation!", | ||
| "🌈 Perfect harmony!", | ||
| ] |
There was a problem hiding this comment.
The docstring suggests this function always prints a success message and dashboard link, but it returns early when USE_COLORS is false and it can also skip printing the link (e.g., empty profile_ids or the dry-run-placeholder sentinel). Please update the docstring to reflect the actual behavior, or adjust the implementation if the intent is to always show a link regardless of color mode.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
1 similar comment
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
| "🌈 Perfect harmony!", | ||
| ] | ||
| print(f"\n{Colors.GREEN}{random.choice(success_msgs)}{Colors.ENDC}") | ||
|
|
Check warning
Code scanning / Prospector (reported by Codacy)
multiple statements on one line (def) (E704) Warning
| "🌈 Perfect harmony!", | ||
| ] | ||
| print(f"\n{Colors.GREEN}{random.choice(success_msgs)}{Colors.ENDC}") | ||
|
|
Check warning
Code scanning / Prospector (reported by Codacy)
ambiguous variable name 'l' (E741) Warning
main.py
Outdated
| ] | ||
| print(f"\n{Colors.GREEN}{random.choice(success_msgs)}{Colors.ENDC}") | ||
|
|
||
| # Construct dashboard URL once, then print it in a single place |
Check warning
Code scanning / Prospector (reported by Codacy)
line too long (268 > 159 characters) (E501) Warning
main.py
Outdated
| ] | ||
| print(f"\n{Colors.GREEN}{random.choice(success_msgs)}{Colors.ENDC}") | ||
|
|
||
| # Construct dashboard URL once, then print it in a single place |
Check warning
Code scanning / Prospector (reported by Codacy)
multiple statements on one line (def) (E704) Warning
| if profile_ids and len(profile_ids) == 1 and profile_ids[0] != "dry-run-placeholder": | ||
| dashboard_url = f"https://controld.com/dashboard/profiles/{profile_ids[0]}/filters" | ||
| elif profile_ids and len(profile_ids) > 1: | ||
| dashboard_url = "https://controld.com/dashboard/profiles" |
Check warning
Code scanning / Prospector (reported by Codacy)
line too long (245 > 159 characters) (E501) Warning
Co-authored-by: abhimehro <84992105+abhimehro@users.noreply.github.com>
|
👋 Development Partner is reviewing this PR. Will provide feedback shortly. |
💡 What: Added a clickable link to the Control D dashboard in the success message.
🎯 Why: Allows users to immediately verify their sync results without navigating manually.
📸 Before/After:
♿ Accessibility: Uses high-contrast cyan color for the link and underline for affordance.
PR created automatically by Jules for task 5279312103953067257 started by @abhimehro