Fix Achiever/Dynamics course pages breaking in local dev - #2608
Open
A-Wheeto wants to merge 4 commits into
Open
Conversation
ENV.fetch("PROXY_URL") returns "" locally (no proxy needed); Faraday tried to parse that empty string as a URI and crashed on nil.hostname. .presence turns it into nil, which Faraday correctly treats as "no proxy."
ENG-2175
Fixtures hadn't been regenerated since the ACHIEVER_USE_LOCAL_TEMPLATES flag was introduced (single commit, years ago) - course/occurrence IDs were stale and didn't resolve against preprod ENG-2175
Rails.cache is a null store in development, so every course-related request round-tripped to preprod live. Added a persistent dev-only file cache. Flips ACHIEVER_USE_LOCAL_TEMPLATES to false by default now that live preprod + caching is the intended default dev path ENG-2175
Achiever::Course::Template::_all passes cache: false to skip Rails.cache (it's wrapped in its own outer cache in prod/staging), but perform_request was checking that flag before the dev file cache branch, so it also skipped the new dev-only cache. Moved the development check ahead of the cache flag so the dev cache applies unconditionally in development, while leaving Rails.cache behaviour in other environments unchanged. ENG-2175
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
PROXY_URL=''(no proxy) was passed straight to Faraday, which tried to parse the empty string as a URI and crashed on nil.hostname. .presence fixes it.ACHIEVER_USE_LOCAL_TEMPLATESflag was introduced. course/occurrence IDs were years stale and didn't resolve against current preprod data.Rails.env.development?only (staging/production/test behavior is unchanged)ACHIEVER_USE_LOCAL_TEMPLATESto false by default (.env.defaults) now that live preprod + caching is the intended default dev path. Anyone without preprod VPN/Dynamics access can set it back to true locally to keep using the static fixtures.