Update CDN URLs in weapon helper functions and constants

[cesaregarza]

This commit modifies the base CDN URL in both the weapon helper functions and constants to include the 'splat-top' subdirectory. Additionally, it changes the file mode of the update_assets.py script to make it executable. These updates ensure consistency in resource paths across the application.

[claude]

Pull Request Review: Update CDN URLs

Summary

This PR updates the base CDN URL across the application to include the splat-top subdirectory path, ensuring consistency in resource paths. The changes are minimal and focused on fixing the CDN path structure.

---

Code Quality and Best Practices

Positive Observations:

• ✅ Consistency: All CDN URL references have been consistently updated to include the /splat-top/ path
• ✅ Centralization: Most URLs are properly centralized through constants (BASE_CDN_URL, WEAPON_INFO_URL, GAME_TRANSLATION_BASE_URL)
• ✅ File permissions: Making scripts/update_assets.py executable is appropriate for a script that should be run directly

Concerns:

1. Hardcoded URL in JavaScript (weapon_helper_functions.js:62):

   • The baseCDNUrl is hardcoded instead of being imported from a centralized constants file
   • This creates a maintenance burden - if the CDN URL changes again, it needs to be updated in multiple places
   • Recommendation: Consider creating a JavaScript constants file similar to the Python constants.py to centralize configuration values

2. URL Duplication:

   • The same base URL is duplicated across files: constants.py has it 3 times (lines 1, 25, 29) and weapon_helper_functions.js has its own copy
   • While constants.py uses BASE_CDN_URL for some operations (via utils.py), the specific data URLs are still hardcoded

---

Potential Bugs or Issues

Critical Finding:

1. Inconsistent URL in constants.py:29:

   • Line 29 uses nyc3.cdn.digitaloceanspaces.com (correct for CDN)
   • However, the old version used nyc3.digitaloceanspaces.com (no .cdn subdomain)
   • This changes the domain, not just the path! This could be intentional (fixing an earlier bug), but should be verified
   • Action Required: Verify that all language files are accessible at the new CDN URL with .cdn subdomain

2. No verification of resource availability:

   • The changes assume all resources exist at the new /splat-top/ path
   • Recommendation: Test that critical resources are accessible:
     • Weapon images (assets/weapon_flat/)
     • Weapon info JSON (data/weapon_info.json)
     • Language files (data/language/)

---

Performance Considerations

• ✅ No performance regression: URL changes shouldn't impact performance
• ✅ Caching is maintained: The caching mechanisms in utils.py (TTLCache) remain unchanged
• ℹ️ CDN benefits: Using the .cdn subdomain (if that was missing before) should actually improve performance through better CDN distribution

---

Security Concerns

1. Minor - HTTPS is maintained: All URLs continue to use HTTPS ✅
2. No injection risks: URLs are static strings, not user-controlled ✅
3. Info disclosure: No sensitive information in the changes ✅

Observation:

• The CDN URLs are public and this appears to be expected behavior for serving static game assets

---

Test Coverage

Major Concern:

• ❌ No tests modified or added: This PR has no test changes
• ❌ No evidence of manual testing: The PR description doesn't mention testing the new URLs

Recommendations:

1. Add integration tests that verify:
   • Weapon images load correctly from the new path
   • weapon_info.json is accessible
   • Language files load for all supported languages
2. Manual testing checklist:
   • Frontend displays weapon images correctly
   • Player detail pages load weapon data
   • All language translations load properly
   • No 404 errors in browser console

Existing test files that may need attention:

• src/react_app/src/App.test.js
• src/react_app/src/components/competition/competitionVizUtils.test.js

---

Additional Recommendations

1. Documentation:

   • Update README.md or other docs if they reference the CDN structure
   • Document the expected CDN directory structure somewhere

2. Deployment Planning:

   • Ensure the DigitalOcean Spaces bucket has the correct directory structure before merging
   • Consider a gradual rollout or feature flag to prevent breaking the production site

3. Backward Compatibility:

   • If the old paths are still in use anywhere, consider keeping them available temporarily
   • Monitor 404 errors after deployment

4. Future Improvement:

   • Create a shared constants file for the React app to avoid hardcoded URLs
   • Consider environment variables for the CDN base URL to support different environments (dev/staging/prod)

---

Files Changed

• ✅ scripts/update_assets.py - File mode change (now executable)
• ✅ src/react_app/src/components/player_components/weapon_helper_functions.js - URL updated
• ✅ src/shared_lib/constants.py - Multiple URL constants updated

---

Verdict

Conditional Approval - The code changes are straightforward and necessary, but I recommend:

1. Before merging: Verify all resources exist at the new CDN paths (especially language files with the new .cdn subdomain)
2. Before merging: Manually test the application to ensure images and data load correctly
3. Follow-up PR: Refactor JavaScript to use centralized constants instead of hardcoded URLs

The changes appear correct for updating the CDN path structure, but the lack of tests and potential domain change for language files warrant careful verification before deployment.

---

Review generated with Claude Code