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: Introduce state and other fields to file
table and implement interfaces
#6523
base: statable
Are you sure you want to change the base?
Conversation
@luke- Question: for version In concrete, this would mean:
Because that is what I actually only urgently need. |
With stable "v1.15" due out soon, I would really want to avoid (larger) changes & features. Basically I am open for new fields like However, it would be important to me that introduced code (APIs/Attributes) are actively used in the core (or official modules). e.g. (just quick thoughts)
Idk if I want quickly introduce these fields that probably won't be used in Core/Modules for a while.
|
@luke- thanks for your comments!
Yes, of course.
Make profile image handling generic and re-usable for any content #6408 would be using the category field for profile images and banners. The above PR would also allow to have multiple profile images. So a further PR could provide the code to actually ...
What would
Yes, that's what we use it for. This code could be provided once we've implemented the sorting. Could also be used to order profile images (see above). On this note: when you previously agreed that "the file module provide a common core API for standard file use cases like
Exactly. PR #6408 stores the following information (although as serialized data, not JSON): {
"data": {
"file._upload.mimetype": "image/jpeg",
"file._upload.hash": "2ef4bfec23ee811fa402de7a7c4d6c48a701439d",
"file._upload.size": 19577,
"file._original.mimetype": "image/jpeg",
"file._original.hash": "ef25b44c8cd3fd88823c5e711fb844195eccb05b",
"file._original.size": 244106,
},
"config": {
"default": null,
}
}
As mentioned above, the code to use those fields is already waiting in PR #6408. It's just that there are some intermediate steps that need to be approved, as we are already in progress of doing.
When you say you're thinking about StateService+Interfaces, do you refer to the general implementation as proposed in PR #6430, or more specific in the sense of applying it to the #6408 uses the (The mentioned PR also allows then to upload a new draft, which will overwrite the main image upon save. Until then, the old original is used. But while this can be done with the file variant, the previous example specifically requires the draft state.) It will - at least in our module - also be used to report, review, ban an image. This could also be used in the core. Also, a user might to make a file temporarily invisible. |
Definitely interesting. Haven't had time to take a closer look yet.
Do you mean images added to a post?
So one user should have multiple profile pictures?
Just an idea to remove this flag. (Which indiciates a file is shown in the file list or not)
Please also see above.
I've we simply add this column it would be fine for me.
I would be interested in possibly using a MySQL JSON column for this. do you have any experience with this?
Both, I'll have to study StateService PR and #6408 more closely for this. Unfortunately, I currently lack the time here. |
@luke- Thanks for your comments! I think we don't need to over-complicate things. Maybe on a general note I'd like to mention that sometimes I receive contradictory requirements for a change to be accepted:
To illustrate this, I'd like ta add my thoughts hen I read your comment above:
-> ok, that means, small changes to me.
-> Seem to be straight-forward...
-> uhm, ok, so not just the fields, but also API and also used in the core (or official modules). So, not so small then?
-> So do you now want to introduce these fields or not? On the forum it gave me the impression that you would want to provide "a common core API for standard file use cases like
-> ... well, I just did what you asked me to do: "So you’re welcome to create PR for this." (same post) So what I've tried to do in my reply to your comment was to indicate that indeed there are use-cases for all the fields I've mentioned and implementation for the core is already underway/waiting for review ... I appreciate that my change requests are complex and far- and deep-reaching. I do my best to present them in a manageable way. Even if sometimes it might not look that way! 🤣 Now, as far as the new fields for
My understanding is, that right now we're only talking about Step 1 and maybe partially Step 2. Maybe we should further discuss scope and details in
Yes, I do have experience with MySQL JSON column, although not used through the Yii DB API. Main points to consider:
|
Part of
FindInstanceInterface
- Part 1.a: Creating base classes #6503FindInstanceInterface
- Part 2: Implementing throughout the code #6463status
tostate
in User, Space, & Membership #6517file
table and implement interfaces #6523 (this PR)