refactor: stop shadowing core REST API templates; inject geometry instead - #385
Merged
Conversation
…tead The plugin shipped full copies of six core *.api.rsb templates (issues, projects, users x index/show) just to add a geojson field (and rotation/distance). Those copies silently drifted from core on every Redmine upgrade: the users/show copy had lost core's '&& !User.current.authorized_by_oauth?' guard on the api_key field, and several newer core user fields (twofa_scheme, passwd_changed_on, status) were missing entirely. Replace the shadowing with an after_action that appends the plugin's own fields to core's rendered JSON/XML response (ApiGeometryInjection). Core renders its template untouched, so the response can no longer fall behind core; the concern never lists a core field. Behavior is preserved: geojson is a parsed object for ?format=json and a JSON string otherwise, present as null/empty when a record has no geometry; projects keep rotation (always) and geojson (only with include=geometry); the issues index keeps distance. The dedicated .geojson FeatureCollection endpoints are unchanged. Verified against the existing issues/users/projects API integration tests (157 assertions) and live in a Redmine 6.1 dev instance across json/xml, show/index, and the empty-geometry case.
There was a problem hiding this comment.
Pull request overview
This PR removes plugin-shipped copies of core REST API *.api.rsb templates (which had drifted from Redmine core) and replaces them with an after_action-based approach that post-processes the rendered JSON/XML to inject the plugin’s geometry-related fields.
Changes:
- Added
ApiGeometryInjectionconcern to mergegeojson(plusrotation/distance) into already-rendered core API responses. - Wired the injection via
after_actionin the issues/projects/users controller patches. - Deleted six previously-shadowed core REST API templates for issues/projects/users index/show.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/redmine_gtt/patches/api_geometry_injection.rb | Adds response post-processing helpers to inject geometry fields into JSON/XML bodies. |
| lib/redmine_gtt/patches/issues_controller_patch.rb | Hooks injection after show/index to append geojson and optional distance. |
| lib/redmine_gtt/patches/projects_controller_patch.rb | Hooks injection after show/index to append rotation and conditional geojson. |
| lib/redmine_gtt/patches/users_controller_patch.rb | Hooks injection after show/index to append geojson without shadowing core user templates. |
| app/views/issues/index.api.rsb | Removes plugin shadow template for issues index API. |
| app/views/issues/show.api.rsb | Removes plugin shadow template for issues show API. |
| app/views/projects/index.api.rsb | Removes plugin shadow template for projects index API. |
| app/views/projects/show.api.rsb | Removes plugin shadow template for projects show API. |
| app/views/users/index.api.rsb | Removes plugin shadow template for users index API. |
| app/views/users/show.api.rsb | Removes plugin shadow template for users show API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
init.rb aliases :geojson to application/json, so request.format.json? is also true for .geojson requests. Without an explicit guard the after_action would reparse and re-serialize the FeatureCollection body the controllers send_data for that format. Skip injection when the format is :geojson.
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
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.
Summary
The plugin shipped full copies of six core
*.api.rsbtemplates (issues,projects,users×index/show) purely to add ageojsonfield (plusrotationon projects anddistanceon the issues index). Copying core's entire field list is update-fragile, and it has already drifted:users/show.api.rsbhad lost core's&& !User.current.authorized_by_oauth?guard on theapi_keyfield (a security-relevant drift), andtwofa_scheme,passwd_changed_on,status) were missing from the response entirely.This replaces the shadowing with an
after_action(ApiGeometryInjection) that appends the plugin's own fields to core's already-rendered JSON/XML response. Core renders its own template untouched, so the API can no longer fall behind core, and the concern never lists a core field.What changed
lib/redmine_gtt/patches/api_geometry_injection.rb: a small concern that merges per-record fields into the rendered resource object(s), parsing/re-serializing JSON or inserting nodes for XML, with a rescue so it can never turn a valid core response into an error.after_actionin the issues/projects/users controller patches.Behavior preserved (backward-compatible)
geojsonis a parsed object for?format=jsonand a JSON string otherwise (xml), and is present asnull/empty when a record has no geometry. Gated ongeom.present?, exactly as the old templates were.rotation(always) andgeojson(only withinclude=geometry).distancewhen present..geojsonFeatureCollection endpoints are unchanged.As a bonus,
usersandissues/projectsresponses now include whatever core fields had drifted out of the stale copies, and theapi_keyOAuth guard is restored.Verification
issues_api_test,users_api_test, andprojects_api_testintegration tests pass (12 runs, 157 assertions, 0 failures) against this change. They pin the exact behavior: object for JSON, string for XML, null/empty when no geometry, for both index and show.include=geometry), user show/index, across.jsonand.xml, plus the empty-geometry case; the.geojsonendpoints still work; server logs clean.