search

SimpleMachines / tools

Tools for SMF: useful scripts, install/repair and others.
21 stars 33 forks source link

Add attachment basedirs to repair_settings #54

Closed BrickOzp closed 2 years ago

BrickOzp commented 2 years ago

Attachment basedir paths also needs to be updated.

Signed-off-by: Oscar Rydhé oscar.rydhe@gmail.com

sbulen commented 2 years ago

Actually, I was just using it on a 2.1 forum and got the following...

Forum was migrated from 2.0, & has never had base directories: image

BrickOzp commented 2 years ago

Thanks for testing @sbulen Turns out there was no check if a setting existed so this error could happen for any of the settings.

sbulen commented 2 years ago

That's because none of the others are optional.

I've used this tool hundreds of times in all different smf versions & have never seen that.

So thanks for fixing!

sbulen commented 2 years ago

As an example, without an avatar url, this is the old behavior: no_avatar_url_old

Vs this is how it behaves with this PR: no_avatar_url_new

sbulen commented 2 years ago

You can either go with a "show it when present" behavior, or maybe a bit better, "show it when basedir specified in the options".

I'm not totally familiar with basedirs, I think they may be required when automanage_attachments isn't empty??? Not sure, that would require some research & testing... image

Note you can also have multiple basedirs, which, to be honest, makes zero sense to me.... The logic only uses one, right? Which one? image

sbulen commented 2 years ago

Works great now, thanks. Let's give it another day or so of airtime to see if anyone else wants to pipe in.

It's possible SleePy would want the version incremented? He has asked for that before. Might be smart to do that before we merge.

jdarwood007 commented 2 years ago

I can send a quick update for the version when we update the downloads page. Its supposed to help us indicate if people have a old version of the tool. Both the date and version are used. Version to indicate revisions to the code, the date to indicate the published date.

sbulen commented 2 years ago

So should we merge this now?