Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enh: [RFC] [WIP] Make profile image handling generic and re-usable for any content #6408

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from

Conversation

martin-rueegg
Copy link
Contributor

@martin-rueegg martin-rueegg commented Jun 28, 2023

The feature to upload and display profile and banner images exists for Users and Spaces. However, this functionality is useful also for other content as well as potentially other content types (once extendable).

This patch aims to make the code re-usable for other content, allowing to customize configuration, actions and permissions based on interfaces and events.

All changes have the streamlining of code in mind and some changes can later be built on, eg. the Interfaces for permission checks. They could be used in other areas of the project or modules.

Current State

Work-in-progress, comments welcome. Still needs testing and potentially some amendments after discussion.

The PR has been broken down in different commit groups:

  1. Enh: Introduce Archiveable, Deletable, Editable, Readable, & Viewable Interfaces #6451
  2. Enh: Introduce FindInstanceInterface - Part 1.a: Creating base classes #6503
  3. Enh: Introduce FindInstanceInterface - Part 2: Implementing throughout the code #6463
  4. Enh Introduce Statable interfaces and implementation #6430
  5. Enh: Rename column status to state in User, Space, & Membership #6517
  6. Some minor changes
  7. Enh: Introduce state and other fields to file table and implement interfaces #6523
  8. Make profile image handling generic and re-usable for any content (this PR)

The CHANGELOG has not been updated in the individual junks for now.

1. Statable interfaces

See #6430 (comment)

2. Introduce Deletable, Editable, Readable, & Viewable Interfaces

See #6451

Also introducing the above mentioned interfaces had the intention to standardize the methods, but also the detectability thereof. It has also been suggested in the code.

3. Minor changes

Some reformatting and small code changes that do not really fit in any of the other chunks

4. Introduce state and other fields to file table

Updating the fields follows up on a forum discussion and also implements the interfaces up until here.

5. Make profile image handling generic and re-usable

Last but not least, making the image upload more generic allows to reuse the code for other post types or attachments, not only for profile images and banners.

About this PR

What kind of change does this PR introduce?

  • Feature
  • Refactor

Does this PR introduce a breaking change? (check one)

  • Should not

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • It's submitted to the develop branch, not the master branch if no hotfix
  • All tests are passing
  • New/updated tests are included
  • Changelog was modified

@luke-
Copy link
Contributor

luke- commented Jun 29, 2023

@martin-rueegg Thanks for the contribution. This looks very interesting.

Unfortunately, extended flexibility often brings a lot of complexity. We always have to weigh things up here.

Please give me some time to take a closer look into the PR/idea.

@martin-rueegg
Copy link
Contributor Author

Hi @luke-

Thank you for your appreciative comment. I'm still working on this as some incompatibilities with the changes that happened during my time-away. I will also try to isolate some changes to the code that are not inseparable from the feature (e.g. introduction of interfaces) into separate commits and add some "documentation".

@martin-rueegg martin-rueegg deleted the enh/content-image branch July 14, 2023 20:43
@martin-rueegg martin-rueegg restored the enh/content-image branch July 14, 2023 20:46
@martin-rueegg martin-rueegg reopened this Jul 14, 2023
@martin-rueegg martin-rueegg force-pushed the enh/content-image branch 2 times, most recently from 8d4f0db to a73a17e Compare July 14, 2023 21:57
@martin-rueegg
Copy link
Contributor Author

Hi @luke-

I've tried to split this mammoth PR into some smaller logical chunks:

  1. Introduce Statable interfaces and implementation
  2. Introduce Deletable, Editable, Readable, & Viewable Interfaces
  3. Some minor changes
  4. Introduce state and other fields to file table and implement interfaces
  5. Make profile image handling generic and re-usable for any content (this PR)

Please have a look at the original post of this PR for further details (under section Current State

Happy to discuss details and how to proceed.

@luke-
Copy link
Contributor

luke- commented Jul 15, 2023

@martin-rueegg Hi Martin, thanks for splitting the PR. It makes it much easier to discuss & review it. I already had a first very quick look at the Statable PR and I really like it. I still need some time to dig deeper into it, but I'll comment directly in the respective PR with my feedback/ideas.

@martin-rueegg martin-rueegg changed the title [Enh] [RFC] [WIP] Make profile image handling generic and re-usable for any content Enh: [RFC] [WIP] Make profile image handling generic and re-usable for any content Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants