Elkarte icon indicating copy to clipboard operation
Elkarte copied to clipboard

[WIP] Attachments

Open emanuele45 opened this issue 2 years ago • 4 comments

It was a while I wanted to kill again the attachments. That's really just a bit of makeup to warm me up. The Attachment class is a placeholder, I just wanted to put these constants somewhere because it always bothered me I could never find what the heck the numbers were. It could hold much more, worst case a good chunk of Attachments.subs.php in a static way.

I was thinking of a couple of things:

  1. drop the storing of the thumbnails in the db (just rely on a prefix to the file name and GTFO),
  2. maybe move the uploaded avatars out of the way (that as far as I remember is a good chunk of annoying code to deal with),
  3. if not too messy, maybe go back to when there was one way to store attachments, just not a single directory, I'd say by months is usually good enough and easy to deal with?

What do you think @Spuds ?

emanuele45 avatar Jan 31 '22 23:01 emanuele45

WooHoo More attachment fun 👍

  1. Not storing the thumbs in the db ... I feel this makes sense. I think the code still does a fileexists after the db call so why not just look for the attachment hash + _thumb. Should save some overhead I think.

  2. I abused the avatar code in a recent commit (no more uploads as attachments, removing all those db calls). Moved all of the avatar upload code out of profile into its own class, and more so just unraveling that messy code. It really did not do what it claimed to do. Anyway any improvements there are welcome.

  3. You mean drop the auto directory options and just make it year/month going forward. That should simplify several areas. I feel some of the current options are a bit confusing and make the UI difficult to follow.

Spuds avatar Feb 01 '22 00:02 Spuds

Dealing with attachments is more of an headache than I remembered. :laughing:

  1. yep, apparently they are already saved with "thumb" but stil lalso in the db with the whole id thingy... confusing.
  2. I saw, didn't notice at first!
  3. yep, I wanted to do it a while ago but gave up. The only downside is that we'd have to reprocess all the attachments during the upgrade... that may become a bit time consuming...

emanuele45 avatar Feb 24 '22 23:02 emanuele45

Codecov Report

Merging #3580 (6bc508f) into development (d3bbbb4) will decrease coverage by 4.13%. The diff coverage is 0.00%.

:exclamation: Current head 6bc508f differs from pull request most recent head 2dcbd0d. Consider uploading reports for the commit 2dcbd0d to get more accurate results

@@                Coverage Diff                @@
##             development    #3580      +/-   ##
=================================================
- Coverage          30.52%   26.39%   -4.14%     
+ Complexity         15723    15534     -189     
=================================================
  Files                423      439      +16     
  Lines              74098    75286    +1188     
=================================================
- Hits               22616    19868    -2748     
- Misses             51482    55418    +3936     
Impacted Files Coverage Δ
sources/ElkArte/Attachments/Download.php 0.00% <0.00%> (ø)
sources/ElkArte/Controller/Attachment.php 26.78% <0.00%> (+16.56%) :arrow_up:
sources/subs/Attachments.subs.php 15.36% <ø> (-2.24%) :arrow_down:
sources/ElkArte/Errors.php 0.00% <0.00%> (-100.00%) :arrow_down:
sources/ElkArte/Modules/Mentions/Post.php 0.00% <0.00%> (-100.00%) :arrow_down:
sources/ElkArte/Modules/Mentions/Display.php 0.00% <0.00%> (-100.00%) :arrow_down:
sources/ElkArte/Notifiers/AbstractNotifier.php 0.00% <0.00%> (-100.00%) :arrow_down:
...urces/ElkArte/UrlGenerator/Standard/ParseQuery.php 0.00% <0.00%> (-100.00%) :arrow_down:
...rces/ElkArte/ScheduledTasks/Tasks/WeeklyDigest.php 0.00% <0.00%> (-100.00%) :arrow_down:
...rces/ElkArte/Modules/Mentions/AbstractMentions.php 0.00% <0.00%> (-90.91%) :arrow_down:
... and 352 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 773df0f...2dcbd0d. Read the comment docs.

codecov-commenter avatar Mar 19 '22 22:03 codecov-commenter

It's an interesting matter that I learned many things from here. Everyone is so co-operative and I would love get the latest news update.

tenbuddy avatar Nov 21 '22 15:11 tenbuddy