🎨 Palette: Smart Profile ID Extraction from URLs#135
Conversation
- Added `extract_profile_id` helper to `main.py` to handle Control D dashboard URLs. - Updated `main()` to use extraction for both CLI arguments and interactive input. - Updated interactive prompt text to inform users they can paste the full URL. - Added comprehensive tests for extraction logic and interactive flow in `test_main.py`. 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. |
|
Merging to
|
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 significantly enhances the user experience of the Palette tool by introducing intelligent parsing for Control D Profile IDs. Previously, users were required to manually extract the alphanumeric ID from a URL, often leading to validation errors. Now, the system automatically extracts the correct ID when a full Control D Dashboard URL is provided, whether through interactive prompts or environment variables. This change reduces cognitive load, prevents common user errors, and streamlines the workflow for obtaining profile IDs. Highlights
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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # Case 8: extract_profile_id correctly extracts ID from URL or returns input | ||
| def test_extract_profile_id(): | ||
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" | ||
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" | ||
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" | ||
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| assert main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" | ||
| assert main.extract_profile_id(None) == "" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Verify sync_profile called with extracted ID | ||
| args, _ = mock_sync.call_args | ||
| assert args[0] == "extracted_id" |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
|
|
||
| # Verify prompt text update | ||
| captured = capsys.readouterr() | ||
| assert "(or just paste the URL)" in captured.out |
Check notice
Code scanning / Bandit
Use of assert detected. The enclosed code will be removed when compiling to optimised byte code. Note test
| if not profile_ids: | ||
| print(f"{Colors.CYAN}ℹ Profile ID is missing.{Colors.ENDC}") | ||
| print(f"{Colors.CYAN} You can find this in the URL of your profile in the Control D Dashboard.{Colors.ENDC}") | ||
| print(f"{Colors.CYAN} You can find this in the URL of your profile in the Control D Dashboard (or just paste the URL).{Colors.ENDC}") |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (146/100) Warning
| assert "Network failure" in str(mock_log.error.call_args) | ||
|
|
||
| # Case 8: extract_profile_id correctly extracts ID from URL or returns input | ||
| def test_extract_profile_id(): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (102/100) Warning test
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" |
Check warning
Code scanning / Pylint (reported by Codacy)
Line too long (102/100) Warning test
| assert main.extract_profile_id(None) == "" | ||
|
|
||
| # Case 9: Interactive input handles URL pasting | ||
| def test_interactive_input_extracts_id(monkeypatch, capsys): |
Check warning
Code scanning / Pylint (reported by Codacy)
Missing function docstring Warning test
| monkeypatch.delenv("TOKEN", raising=False) | ||
|
|
||
| # Reload main with isatty=True | ||
| m = reload_main_with_env(monkeypatch, isatty=True) |
Check warning
Code scanning / Pylint (reported by Codacy)
Variable name "m" doesn't conform to snake_case naming style Warning test
| if not profile_ids: | ||
| print(f"{Colors.CYAN}ℹ Profile ID is missing.{Colors.ENDC}") | ||
| print(f"{Colors.CYAN} You can find this in the URL of your profile in the Control D Dashboard.{Colors.ENDC}") | ||
| print(f"{Colors.CYAN} You can find this in the URL of your profile in the Control D Dashboard (or just paste the URL).{Colors.ENDC}") |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (146/100) Warning
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (102/100) Warning test
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" |
Check warning
Code scanning / Pylintpython3 (reported by Codacy)
Line too long (102/100) Warning test
| # Case 8: extract_profile_id correctly extracts ID from URL or returns input | ||
| def test_extract_profile_id(): | ||
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" |
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
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" |
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
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" | ||
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" |
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
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" |
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
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" | ||
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" |
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
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" | ||
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" |
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
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" |
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 main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" | ||
| assert main.extract_profile_id(None) == "" |
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
|
|
||
| # Verify sync_profile called with extracted ID | ||
| args, _ = mock_sync.call_args | ||
| assert args[0] == "extracted_id" |
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
|
|
||
| # Verify prompt text update | ||
| captured = capsys.readouterr() | ||
| assert "(or just paste the URL)" in captured.out |
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
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a smart profile ID extraction feature, allowing users to paste full Control D dashboard URLs instead of just the ID. This is a great usability improvement. The implementation is clean and the new logic is well-covered by tests. I've added a couple of minor suggestions: one to improve type hinting for better code correctness and another to refactor a test for better maintainability using pytest.parametrize. Overall, this is a solid contribution.
|
|
||
| return True | ||
|
|
||
| def extract_profile_id(text: str) -> str: |
There was a problem hiding this comment.
The type hint for text is str, but the function logic and tests handle None as a valid input. To make the function signature more accurate and prevent potential type checker issues, it's better to use Optional[str]. You'll need to ensure from typing import Optional is present.
| def extract_profile_id(text: str) -> str: | |
| def extract_profile_id(text: Optional[str]) -> str: |
| def test_extract_profile_id(): | ||
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" | ||
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" | ||
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" | ||
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" | ||
| assert main.extract_profile_id(None) == "" |
There was a problem hiding this comment.
This test function contains multiple assert statements for different test cases. To improve readability and make it easier to add new test cases in the future, consider refactoring it to use pytest.mark.parametrize. This will also provide more granular feedback if one of the cases fails.
| def test_extract_profile_id(): | |
| # Regular ID | |
| assert main.extract_profile_id("12345") == "12345" | |
| # URL with /filters | |
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" | |
| # URL without /filters | |
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | |
| # URL with params | |
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" | |
| # Clean up whitespace | |
| assert main.extract_profile_id(" 12345 ") == "12345" | |
| # Invalid input returns as is (cleaned) | |
| assert main.extract_profile_id("random-string") == "random-string" | |
| # Empty input | |
| assert main.extract_profile_id("") == "" | |
| assert main.extract_profile_id(None) == "" | |
| @pytest.mark.parametrize( | |
| "input_text, expected_id", | |
| [ | |
| ("12345", "12345"), | |
| ("https://controld.com/dashboard/profiles/12345/filters", "12345"), | |
| ("https://controld.com/dashboard/profiles/12345", "12345"), | |
| ("https://controld.com/dashboard/profiles/12345?foo=bar", "12345"), | |
| (" 12345 ", "12345"), | |
| ("random-string", "random-string"), | |
| ("", ""), | |
| (None, ""), | |
| ], | |
| ) | |
| def test_extract_profile_id(input_text, expected_id): | |
| assert main.extract_profile_id(input_text) == expected_id |
There was a problem hiding this comment.
Pull request overview
This PR adds smart URL parsing capability to extract profile IDs from full Control D Dashboard URLs, reducing user friction when providing profile IDs.
Changes:
- Added
extract_profile_id()function that uses regex to extract profile IDs from URLs or returns the input as-is - Applied extraction logic to all profile ID input points (environment variables, CLI arguments, and interactive prompts)
- Updated interactive prompt text to explicitly mention URL pasting support
- Added comprehensive test coverage for the new functionality
- Documented the learning in palette.md
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.py | Added extract_profile_id() function and integrated it into profile ID processing logic; updated interactive prompt text |
| test_main.py | Added unit tests for extract_profile_id() covering multiple URL formats and edge cases; added integration test verifying URL extraction in interactive mode |
| .Jules/palette.md | Documented the learning and action for smart input extraction pattern |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| text = text.strip() | ||
| # Pattern for Control D Dashboard URLs | ||
| # e.g. https://controld.com/dashboard/profiles/12345abc/filters | ||
| match = re.search(r"controld\.com/dashboard/profiles/([^/?#\s]+)", text) |
There was a problem hiding this comment.
The regex pattern for extracting profile IDs from URLs is case-sensitive and will not match URLs with uppercase letters in the domain (e.g., "https://ControlD.com/..." or "HTTPS://CONTROLD.COM/..."). Consider making the pattern case-insensitive by adding the re.IGNORECASE flag or by explicitly handling both cases in the pattern. Domain names are case-insensitive per RFC standards, so users might paste URLs with different casing.
| match = re.search(r"controld\.com/dashboard/profiles/([^/?#\s]+)", text) | |
| match = re.search(r"controld\.com/dashboard/profiles/([^/?#\s]+)", text, re.IGNORECASE) |
| def test_extract_profile_id(): | ||
| # Regular ID | ||
| assert main.extract_profile_id("12345") == "12345" | ||
| # URL with /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345/filters") == "12345" | ||
| # URL without /filters | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345") == "12345" | ||
| # URL with params | ||
| assert main.extract_profile_id("https://controld.com/dashboard/profiles/12345?foo=bar") == "12345" | ||
| # Clean up whitespace | ||
| assert main.extract_profile_id(" 12345 ") == "12345" | ||
| # Invalid input returns as is (cleaned) | ||
| assert main.extract_profile_id("random-string") == "random-string" | ||
| # Empty input | ||
| assert main.extract_profile_id("") == "" | ||
| assert main.extract_profile_id(None) == "" |
There was a problem hiding this comment.
Add a test case for URLs with uppercase letters in the domain (e.g., "HTTPS://CONTROLD.COM/dashboard/profiles/12345/filters" or "https://ControlD.com/dashboard/profiles/12345"). Domain names are case-insensitive per RFC standards, but the current regex pattern is case-sensitive and may not handle these cases correctly.
abhimehro
deleted the
palette-smart-profile-id-extraction-4363998502891798168
branch
🎨 Palette: Smart Profile ID Extraction
💡 What:
Added smart extraction logic that allows users to paste the full Control D Dashboard URL (e.g.,
https://controld.com/dashboard/profiles/12345/filters) when prompted for a Profile ID. The system automatically extracts the ID (12345) and proceeds.🎯 Why:
Users often copy the full URL from their browser address bar. Previously, this caused a validation error because the script expected only the alphanumeric ID. This change removes that friction and prevents manual copy-paste errors.
♿ Accessibility/UX:
PR created automatically by Jules for task 4363998502891798168 started by @abhimehro