Skip to content

fix: clear generated code after composer require#81

Merged
dgjlindsay merged 3 commits intomainfrom
doug/fix-dev-tooling
Apr 14, 2026
Merged

fix: clear generated code after composer require#81
dgjlindsay merged 3 commits intomainfrom
doug/fix-dev-tooling

Conversation

@dgjlindsay
Copy link
Copy Markdown
Contributor

Summary

  • Clear stale generated code after composer require during make install
  • The base Docker image ships pre-compiled DI classes that conflict with the plugin's classes after composer install, causing DI compilation errors

Test plan

  • make install completes without DI compilation errors
  • make test passes

🤖 Generated with Claude Code

Clear stale generated code after composer require during install.
Fix proxy base_link_url not being set in "apply immediately" section,
causing navigation links to redirect back to localhost.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No critical issues. The fix correctly clears stale pre-compiled DI classes from the base Docker image before setup:di:compile runs, which is the standard Magento approach for this problem. Ready for human approval.

dgjlindsay and others added 2 commits April 14, 2026 11:43
Port from magento-abn-plugin PR #10. Adds getBrand() and getBrandVersion()
to Repository, appends brand query params to payment URL, terms link, and
sole trader iframe. Defaults empty (no brand override for Two). Forks
override via TWO_BRAND / TWO_BRAND_VERSION env vars in .env.local.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +626 to +630
var brandParams = config.brand ? `&brand=${config.brand}` : '';
if (config.brandVersion) {
brandParams += `&brandVersion=${config.brandVersion}`;
}
const URL = `${config.checkoutPageUrl}/soletrader/signup?businessToken=${this.delegationToken}&autofillToken=${this.autofillToken}&autofillData=${data}${brandParams}`;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brand values are interpolated directly into the URL without encoding. The PHP side correctly uses http_build_query(), but if these env vars ever contain spaces or special chars the URL here will be malformed. Use encodeURIComponent for consistency.

Suggested change
var brandParams = config.brand ? `&brand=${config.brand}` : '';
if (config.brandVersion) {
brandParams += `&brandVersion=${config.brandVersion}`;
}
const URL = `${config.checkoutPageUrl}/soletrader/signup?businessToken=${this.delegationToken}&autofillToken=${this.autofillToken}&autofillData=${data}${brandParams}`;
var brandParams = config.brand ? '&brand=' + encodeURIComponent(config.brand) : '';
if (config.brandVersion) {
brandParams += '&brandVersion=' + encodeURIComponent(config.brandVersion);
}
const URL = `${config.checkoutPageUrl}/soletrader/signup?businessToken=${this.delegationToken}&autofillToken=${this.autofillToken}&autofillData=${data}${brandParams}`;

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewing after new commits. One minor correctness issue flagged inline (missing encodeURIComponent in JS brand params). The core fix (rm -rf /data/generated/code) and the PHP brand param logic are correct. Ready for human approval once the encoding is addressed.

@dgjlindsay dgjlindsay merged commit f4e32ca into main Apr 14, 2026
21 checks passed
@dgjlindsay dgjlindsay deleted the doug/fix-dev-tooling branch April 14, 2026 13:38
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