-
-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add command to copy files from vendor to theme (#133) #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dermatz
wants to merge
26
commits into
main
Choose a base branch
from
110-feature-request-copy-vendor-files-to-theme
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
0109fc5
feat: add command to copy files from vendor to theme
dermatz 40b62e2
feat: add method to retrieve theme by code
dermatz 8cb4e6d
fix: return proper CLI constants for failure and success cases
dermatz 2e7968f
feat: enhance theme path resolution using ComponentRegistrar
dermatz af10b97
feat: add PHPUnit workflow and unit tests for VendorFileMapper
dermatz 0d7f349
feat: improve file copy command with error handling and path normaliz…
dermatz 4b8b47d
refactor: remove unused filesystem dependency and update type casting
dermatz 37a421c
feat: add tests for copy from vendor functionality in workflows
dermatz 3043585
feat: add ES_SKIP_CGROUPS_CHECK environment variable for Elasticsearch
dermatz 38c12fc
fix: update Elasticsearch configuration for health checks
dermatz e057e78
feat: update Elasticsearch version and add new theme copy commands
dermatz 0933eba
feat: add dry-run option to copy-from-vendor command for previews
dermatz 0f429be
feat: update test for theme copy from vendor with real module template
dermatz 5d0cc83
feat: simplify test for copy from vendor command in functional tests
dermatz a28eb2b
refactor: reorder detection patterns in VendorFileMapper for clarity
dermatz ca088d1
Apply suggestion from @Copilot
dermatz 8df5fd0
Update src/Console/Command/Theme/CopyFromVendorCommand.php
dermatz ee34736
Update .github/workflows/functional-tests.yml
dermatz 323447a
Update src/Service/VendorFileMapper.php
dermatz 04e78e6
Update src/Console/Command/Theme/CopyFromVendorCommand.php
dermatz 869537a
feat: add method to retrieve theme by code in ThemeList
dermatz 3847442
feat: enhance VendorFileMapper with theme area validation and extraction
dermatz 0c2eeb2
feat: implement copy command tests and update VendorFileMapper
dermatz aa6e8a5
docs: update testing guide for copy command with new test cases
dermatz 49df7c8
test: enhance copy command tests with dynamic file discovery
dermatz a123d0b
feat: enhance file copy confirmation with improved prompt handling
dermatz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,15 +32,15 @@ jobs: | |
| - 3306:3306 | ||
| options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 | ||
|
|
||
| opensearch: | ||
| image: opensearchproject/opensearch:2.11.0 | ||
| ports: | ||
| - 9200:9200 | ||
| elasticsearch: | ||
| image: elasticsearch:7.17.25 | ||
| env: | ||
| discovery.type: single-node | ||
| plugins.security.disabled: true | ||
| OPENSEARCH_JAVA_OPTS: -Xms512m -Xmx512m | ||
| options: --health-cmd="curl http://localhost:9200/_cluster/health" --health-interval=10s --health-timeout=5s --health-retries=10 | ||
| xpack.security.enabled: false | ||
dermatz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ES_JAVA_OPTS: "-Xms512m -Xmx512m" | ||
| ports: | ||
| - 9200:9200 | ||
| options: --health-cmd="curl -s http://localhost:9200/_cluster/health" --health-interval=10s --health-timeout=5s --health-retries=10 | ||
|
Comment on lines
+35
to
+43
|
||
|
|
||
| steps: | ||
| - name: Checkout code | ||
|
|
@@ -136,22 +136,103 @@ jobs: | |
| bin/magento mageforge:theme:inspector --help | ||
| bin/magento mageforge:hyva:compatibility:check --help | ||
| bin/magento mageforge:hyva:tokens --help | ||
| bin/magento mageforge:theme:copy-from-vendor --help | ||
|
|
||
| echo "Verify command aliases work:" | ||
| bin/magento m:s:v --help | ||
| bin/magento m:s:c --help | ||
| bin/magento m:t:l --help | ||
| bin/magento m:t:b --help | ||
| bin/magento m:t:w --help | ||
| bin/magento m:t:c --help | ||
| bin/magento m:h:c:c --help | ||
| bin/magento frontend:list --help | ||
| bin/magento frontend:build --help | ||
| bin/magento frontend:watch --help | ||
| bin/magento frontend:clean --help | ||
| bin/magento theme:copy --help | ||
| bin/magento hyva:check --help | ||
| bin/magento hyva:tokens --help | ||
|
|
||
| - name: Test Copy Command Functionality | ||
| working-directory: magento2 | ||
| run: | | ||
| echo "Finding available test files..." | ||
|
|
||
| # Find a frontend template file from any core module | ||
| FRONTEND_FILE=$(find vendor/magento/module-*/view/frontend/templates -type f -name "*.phtml" 2>/dev/null | head -1) | ||
| if [ -z "$FRONTEND_FILE" ]; then | ||
| echo "No frontend template files found, trying layout files..." | ||
| FRONTEND_FILE=$(find vendor/magento/module-*/view/frontend/layout -type f -name "*.xml" 2>/dev/null | head -1) | ||
| fi | ||
|
|
||
| # Find a base template file | ||
| BASE_FILE=$(find vendor/magento/module-*/view/base/templates -type f -name "*.phtml" 2>/dev/null | head -1) | ||
| if [ -z "$BASE_FILE" ]; then | ||
| echo "No base template files found, trying web files..." | ||
| BASE_FILE=$(find vendor/magento/module-*/view/base/web -type f \( -name "*.js" -o -name "*.css" \) 2>/dev/null | head -1) | ||
| fi | ||
|
|
||
| # Find an etc file for negative test | ||
| ETC_FILE=$(find vendor/magento/module-catalog/etc -type f -name "*.xml" 2>/dev/null | head -1) | ||
|
|
||
| echo "Test files found:" | ||
| echo "Frontend: $FRONTEND_FILE" | ||
| echo "Base: $BASE_FILE" | ||
| echo "Etc: $ETC_FILE" | ||
| echo "" | ||
|
|
||
| if [ -n "$FRONTEND_FILE" ]; then | ||
| echo "Test 1: Copy frontend file to frontend theme (dry-run):" | ||
| bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$FRONTEND_FILE" \ | ||
| Magento/luma \ | ||
| --dry-run | ||
| echo "✓ Frontend to frontend mapping works" | ||
| echo "" | ||
| else | ||
| echo "Warning: No frontend file found for testing" | ||
| fi | ||
|
|
||
| if [ -n "$BASE_FILE" ]; then | ||
| echo "Test 2: Copy base file to frontend theme (dry-run):" | ||
| bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$BASE_FILE" \ | ||
| Magento/blank \ | ||
| --dry-run | ||
| echo "✓ Base to frontend mapping works" | ||
| echo "" | ||
| else | ||
| echo "Warning: No base file found for testing" | ||
| fi | ||
|
|
||
| if [ -n "$ETC_FILE" ]; then | ||
| echo "Test 3: Verify non-view file is rejected:" | ||
| if bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$ETC_FILE" \ | ||
| Magento/luma \ | ||
| --dry-run 2>&1 | grep -q "not under a view"; then | ||
| echo "✓ Non-view file correctly rejected" | ||
| else | ||
| echo "✗ Non-view file validation failed" | ||
| exit 1 | ||
| fi | ||
| echo "" | ||
| else | ||
| echo "Warning: No etc file found for negative testing" | ||
| fi | ||
|
|
||
| if [ -n "$FRONTEND_FILE" ]; then | ||
| echo "Test 4: Verify cross-area mapping is rejected (frontend -> adminhtml):" | ||
| if bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$FRONTEND_FILE" \ | ||
| Magento/backend \ | ||
| --dry-run 2>&1 | grep -q "Cannot map file from area"; then | ||
| echo "✓ Cross-area mapping correctly rejected" | ||
| else | ||
| echo "✗ Cross-area validation failed" | ||
| exit 1 | ||
| fi | ||
| echo "" | ||
| else | ||
| echo "Warning: No frontend file found for cross-area testing" | ||
| fi | ||
|
|
||
| echo "✓ All copy command tests passed!" | ||
|
|
||
| - name: Test Summary | ||
| run: | | ||
| echo "MageForge module compatibility test with Magento ${{ matrix.magento-version }} completed" | ||
|
|
@@ -274,22 +355,103 @@ jobs: | |
| bin/magento mageforge:theme:inspector --help | ||
| bin/magento mageforge:hyva:compatibility:check --help | ||
| bin/magento mageforge:hyva:tokens --help | ||
| bin/magento mageforge:theme:copy-from-vendor --help | ||
|
|
||
| echo "Verify command aliases work:" | ||
| bin/magento m:s:v --help | ||
| bin/magento m:s:c --help | ||
| bin/magento m:t:l --help | ||
| bin/magento m:t:b --help | ||
| bin/magento m:t:w --help | ||
| bin/magento m:t:c --help | ||
| bin/magento m:h:c:c --help | ||
| bin/magento frontend:list --help | ||
| bin/magento frontend:build --help | ||
| bin/magento frontend:watch --help | ||
| bin/magento frontend:clean --help | ||
| bin/magento theme:copy --help | ||
| bin/magento hyva:check --help | ||
| bin/magento hyva:tokens --help | ||
|
|
||
| - name: Test Copy Command Functionality | ||
| working-directory: magento2 | ||
| run: | | ||
| echo "Finding available test files..." | ||
|
|
||
| # Find a frontend template file from any core module | ||
| FRONTEND_FILE=$(find vendor/magento/module-*/view/frontend/templates -type f -name "*.phtml" 2>/dev/null | head -1) | ||
| if [ -z "$FRONTEND_FILE" ]; then | ||
| echo "No frontend template files found, trying layout files..." | ||
| FRONTEND_FILE=$(find vendor/magento/module-*/view/frontend/layout -type f -name "*.xml" 2>/dev/null | head -1) | ||
| fi | ||
|
|
||
| # Find a base template file | ||
| BASE_FILE=$(find vendor/magento/module-*/view/base/templates -type f -name "*.phtml" 2>/dev/null | head -1) | ||
| if [ -z "$BASE_FILE" ]; then | ||
| echo "No base template files found, trying web files..." | ||
| BASE_FILE=$(find vendor/magento/module-*/view/base/web -type f \( -name "*.js" -o -name "*.css" \) 2>/dev/null | head -1) | ||
| fi | ||
|
|
||
| # Find an etc file for negative test | ||
| ETC_FILE=$(find vendor/magento/module-catalog/etc -type f -name "*.xml" 2>/dev/null | head -1) | ||
|
|
||
| echo "Test files found:" | ||
| echo "Frontend: $FRONTEND_FILE" | ||
| echo "Base: $BASE_FILE" | ||
| echo "Etc: $ETC_FILE" | ||
| echo "" | ||
|
|
||
| if [ -n "$FRONTEND_FILE" ]; then | ||
| echo "Test 1: Copy frontend file to frontend theme (dry-run):" | ||
| bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$FRONTEND_FILE" \ | ||
| Magento/luma \ | ||
| --dry-run | ||
| echo "✓ Frontend to frontend mapping works" | ||
| echo "" | ||
| else | ||
| echo "Warning: No frontend file found for testing" | ||
| fi | ||
|
|
||
| if [ -n "$BASE_FILE" ]; then | ||
| echo "Test 2: Copy base file to frontend theme (dry-run):" | ||
| bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$BASE_FILE" \ | ||
| Magento/blank \ | ||
| --dry-run | ||
| echo "✓ Base to frontend mapping works" | ||
| echo "" | ||
| else | ||
| echo "Warning: No base file found for testing" | ||
| fi | ||
|
|
||
| if [ -n "$ETC_FILE" ]; then | ||
| echo "Test 3: Verify non-view file is rejected:" | ||
| if bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$ETC_FILE" \ | ||
| Magento/luma \ | ||
| --dry-run 2>&1 | grep -q "not under a view"; then | ||
| echo "✓ Non-view file correctly rejected" | ||
| else | ||
| echo "✗ Non-view file validation failed" | ||
| exit 1 | ||
| fi | ||
| echo "" | ||
| else | ||
| echo "Warning: No etc file found for negative testing" | ||
| fi | ||
|
|
||
| if [ -n "$FRONTEND_FILE" ]; then | ||
| echo "Test 4: Verify cross-area mapping is rejected (frontend -> adminhtml):" | ||
| if bin/magento mageforge:theme:copy-from-vendor \ | ||
| "$FRONTEND_FILE" \ | ||
| Magento/backend \ | ||
| --dry-run 2>&1 | grep -q "Cannot map file from area"; then | ||
| echo "✓ Cross-area mapping correctly rejected" | ||
| else | ||
| echo "✗ Cross-area validation failed" | ||
| exit 1 | ||
| fi | ||
| echo "" | ||
| else | ||
| echo "Warning: No frontend file found for cross-area testing" | ||
| fi | ||
|
|
||
| echo "✓ All copy command tests passed!" | ||
|
|
||
| - name: Test Summary | ||
| run: | | ||
| echo "MageForge module compatibility test with Magento 2.4.8 completed" | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test commands at lines 168-172 will fail because the 'file' argument is required (InputArgument::REQUIRED), but the test runs the command with only the --dry-run option and no file argument. Magento's Symfony Console will throw an error before executeCommand() is reached. The test should either provide a valid file path or expect the specific validation error message.