snoopdave commented on PR #217: URL: https://github.com/apache/roller/pull/217#issuecomment-6253742736
🐞Claude Issue: **PR-Review: General Issues** These are outside the diff hunks: - **Important:** `MediaCollection.java:155` (AtomPub media upload) still sets the original path, from the request path. This PR makes the original path something only theme import and the editor (when `mediafile.originalPathEdit.enabled=true`) set. The AtomPub value is also odd: `justPath` is the last path segment (the directory, for example `/images`), not the file's path, so every file uploaded to one directory gets the same original path, and `getMediaFileByOriginalPath` can then match more than one row. Suggest removing `mf.setOriginalPath(justPath)` there, or setting it the way `ThemeManagerImpl` does (`dir + "/" + name`) only if AtomPub clients need it. - **Important:** No test covers the new `MediaFileEdit.save` behaviour: the original path is kept when `mediafile.originalPathEdit.enabled` is false and set when it is true. `PreviewResourceServlet` has the same change as `ResourceServlet` but no test; a small `PreviewResourceServletTest` mirroring `ResourceServletTest` would cover both lookups (preview theme and weblog theme). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
