-
-
Notifications
You must be signed in to change notification settings - Fork 6
Fix file upload URLs to honor updateEndpoint configuration #208
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
Conversation
Bumps [elpete/setup-commandbox](https://github.com/elpete/setup-commandbox) from 1.0.0 to 1.0.1. - [Release notes](https://github.com/elpete/setup-commandbox/releases) - [Commits](elpete/setup-commandbox@v1.0.0...v1.0.1) --- updated-dependencies: - dependency-name: elpete/setup-commandbox dependency-version: 1.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…s/elpete/setup-commandbox-1.0.1 Bump elpete/setup-commandbox from 1.0.0 to 1.0.1
|
@grantcopley 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs. I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review. |
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
|
@copilot we need to target this PR to the next branch not development |
I understand you want to target the "next" branch instead of "development". Since this is a PR targeting change rather than a code change, you'll need to update the base branch in the GitHub UI:
If there are any merge conflicts after changing the base branch, I can help resolve those once the retargeting is complete. |
|
@copilot we have conflicts |
Co-authored-by: grantcopley <1197835+grantcopley@users.noreply.github.com>
Conflicts resolved! I've rebased the changes onto the "next" branch structure. The implementation maintains the same functionality:
The fix now works correctly with the next branch structure. Commit: 5ecf734 |
File Upload URL Configuration Fix ✅
Successfully resolved merge conflicts after retargeting PR to the "next" branch. The implementation now correctly applies to the next branch structure with all functionality preserved.
Implementation Complete ✅
generateSignedUploadURL()to use the configured endpointConflict Resolution
The conflicts arose because the "next" branch had a different structure than "development":
generateSignedUploadURL()was at different line numbers (531-541 vs 446-456)getUpdateEndpoint()was at different line numbers (614-617 vs 529-532)Resolution applied:
getUploadEndpoint()method at line 624 aftergetUpdateEndpoint()generateSignedUploadURL()at line 540 to usegetUploadEndpoint()Changes Made for Next Branch
1. Added
getUploadEndpoint()method at line 624 inCBWIREController.cfcfunction getUploadEndpoint() { var updateEndpoint = getUpdateEndpoint(); // Replace "/update" with "/upload" in the endpoint return updateEndpoint.replace("/update", "/upload"); }2. Updated
generateSignedUploadURL()method at line 540Before: Hardcoded
/cbwire/uploadAfter: Uses configured endpoint
3. Added comprehensive tests in CBWIRESpec.cfc
Problem Solved ✅
Root Issue:
generateSignedUploadURL()hardcoded/cbwire/uploadregardless ofupdateEndpointconfigurationSolution: Now properly derives upload endpoint from the configured update endpoint setting
Example Behavior:
updateEndpoint = "/index.bxm/cbwire/update"→uploadEndpoint = "/index.bxm/cbwire/upload"✅updateEndpoint = "/cbwire/update"→uploadEndpoint = "/cbwire/upload"✅Conflicts resolved and implementation is minimal, surgical, and precise - exactly what was needed to fix the specific issue without disrupting existing functionality.
Fixes #207.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.