Skip to content

Conversation

@wrandall22
Copy link
Contributor

@wrandall22 wrandall22 commented Dec 5, 2025

Description

Rollbar

The AEM reverse replication process cannot handle files that are too large. It converts the image bytes into JSON, which adds some bloat to the file, which is why I'm using 15MB here instead of 20MB.

Testing

  • Go to the designation editor page
  • Click on the cover photo editor
  • Browse to a file that is larger than 15MB
  • You should see an error message telling you the image is too large
  • Add a file that is less than 15MB
  • You should see success, and the error message should be gone

Do the same steps above with the carousel editor.

Checklist:

  • I have given my PR a title with the format "EP-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels (Add the label "On Staging" to get the branch automatically merged into staging)
  • I have requested a review from another person on the project
  • I have checked that stage-branch-merger successfully merged my branch to staging or manually merged it myself
  • I have tested my changes in staging for regular checkout
  • I have tested my changes in staging for branded checkout

@wrandall22 wrandall22 requested a review from canac December 5, 2025 14:21
@wrandall22 wrandall22 added the On Staging Will be merged into the staging branch by GitHub Actions label Dec 5, 2025
@stage-branch-merger
Copy link

I see you added the "On Staging" label, I'll get this merged to the staging branch!

@canac
Copy link
Contributor

canac commented Dec 5, 2025

Code looks good, but staging isn't updating because CI is failing: https://github.com/CruGlobal/give-web/actions/runs/19965823839/job/57257611535?pr=1259

It looks like jonshaffer deleted the angular-environment repo we depend on:

"angular-environment": "https://github.com/jonshaffer/angular-environment.git#d3082c06fb16804d324faac9b7e753fd64a44e5d",

@wrandall22
Copy link
Contributor Author

wrandall22 commented Dec 5, 2025

I see a different one here: https://www.npmjs.com/package/angular-environment

@wrandall22
Copy link
Contributor Author

It looks like the main reason for switching was to remove a console.log statement; maybe worth going back

@wrandall22 wrandall22 force-pushed the prevent-too-large-images branch from fa41ece to fd853a6 Compare December 5, 2025 16:12
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

We're on code freeze, so this can't be merged for another month or so, but it looks great! Thanks for the improvement!

@wrandall22
Copy link
Contributor Author

I might ask Mike Nelson about that; quite a few people are trying to update their giving pages to handle end of year giving and are running into this error with zero feedback from our system on why they can't upload an image. This is completely separate from the checkout process so he might be willing to let it go through.

@canac
Copy link
Contributor

canac commented Dec 5, 2025

I might ask Mike Nelson about that

Oh OK! If Mike is good with it then I am. Have you talked with him about this issue? If you haven't yet I can reach out unless you'd like to.

@wrandall22
Copy link
Contributor Author

I reached out to him today, he is fine with it going live during code freeze since it has no impact on checkout and the risk is low.

@canac
Copy link
Contributor

canac commented Dec 5, 2025

I reached out to him today, he is fine with it going live during code freeze since it has no impact on checkout and the risk is low.

The only risk in my mind is switching out the angular-environment package source, but I compared the dist files for the old and new version and they are identical, so we should be OK.

A Monday deployment might be wise, but feel free to deploy whenever.

@wrandall22
Copy link
Contributor Author

I'm going to go ahead and merge. I keep getting these errors coming through, and there is plenty of time to rollback today if needed.

@wrandall22 wrandall22 merged commit 5e660cf into master Dec 5, 2025
5 checks passed
@wrandall22 wrandall22 deleted the prevent-too-large-images branch December 5, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

On Staging Will be merged into the staging branch by GitHub Actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants