Skip to content

refactor(form): improve _getUpload and remove unused getUploadedFileType#30

Merged
ralflang merged 1 commit into
horde:FRAMEWORK_6_0from
horde6:refactor_image
May 23, 2026
Merged

refactor(form): improve _getUpload and remove unused getUploadedFileType#30
ralflang merged 1 commit into
horde:FRAMEWORK_6_0from
horde6:refactor_image

Conversation

@amulet1
Copy link
Copy Markdown
Member

@amulet1 amulet1 commented May 10, 2026

  • Separate upload handling from try/catch: file-not-uploaded logic
  • Build $img as a local array and assign to $this->_img at the end, rather than mutating $this->_img piecemeal throughout the method
  • Remove getUploadedFileType static method, inline/improve its logic
  • Related code improvements

- Separate upload handling from try/catch: file-not-uploaded logic
- Build $img as a local array and assign to $this->_img at the end,
  rather than mutating $this->_img piecemeal throughout the method
- Remove getUploadedFileType static method, inline/improve its logic
- Related code improvements
@amulet1 amulet1 requested a review from TDannhauer May 10, 2026 20:37
Copy link
Copy Markdown
Contributor

@TDannhauer TDannhauer left a comment

Choose a reason for hiding this comment

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

Looks like a solid cleanup: clearer success vs. non-upload paths, single assignment into $this->_img['img'], and MIME handling after the file is moved to temp storage.

BC: getUploadedFileType was removed as a public static method. Is that intentional for FRAMEWORK_6_0? If anything outside this package still calls it, we should either keep a deprecated stub or document the break in the changelog / upgrade notes.

Tests: Could you add or extend tests for this flow (happy path upload, no new file with existing hash, explicit remove)? Even a small unit/integration test would help lock in behavior after this refactor.

@TDannhauer TDannhauer requested a review from ralflang May 11, 2026 10:59
@TDannhauer
Copy link
Copy Markdown
Contributor

@ralflang what do you think about the BC break. is it okay?

Comment thread lib/Horde/Form/Type.php
Comment thread lib/Horde/Form/Type.php
@amulet1
Copy link
Copy Markdown
Member Author

amulet1 commented May 11, 2026

Note, the getUploadedFileType() was removed after I confirmed that it is not being used any other Horde modules.

In general, other places should rely on getImage() or getInfo() - and it looks like they do.

@amulet1 amulet1 requested a review from ralflang May 13, 2026 17:40
@ralflang ralflang changed the title Improve _getUpload, remove getUploadedFileType method refactor(form): improve _getUpload and remove unused getUploadedFileType May 22, 2026
@ralflang ralflang merged commit ee0f67c into horde:FRAMEWORK_6_0 May 23, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants