Implemented [Bugfix] Banner files deletion on user deletion

Discussion in 'Core Feature Requests' started by Daegaladh, Aug 26, 2016.

  1. Daegaladh

    Daegaladh Member

    I've noticed that when you delete a user, some banner files still remain, that's because the if-elseif structure, since the user can upload a new banner with a different extension, those files are never deleted. Also premium banners are never deleted either.

    That's why I propose this fix:

    in sources/admin/delete.php
    Replace:
    PHP:
        if(file_exists("banners/{$username}.jpg"))
        {
            
    unlink("banners/{$username}.jpg");
        }
        else if(
    file_exists("banners/{$username}.jpeg"))
        {
            
    unlink("banners/{$username}.jpeg");
        }
        else if(
    file_exists("banners/{$username}.png"))
        {
            
    unlink("banners/{$username}.png");
        }
        else if(
    file_exists("banners/{$username}.gif"))
        {
            
    unlink("banners/{$username}.gif");
    With:
    PHP:
        foreach (glob("banners/{$username}.{jpg,jpeg,png,gif}"GLOB_BRACE) as $banner)
            
    unlink($banner);
        foreach (
    glob("banners/premium_{$username}.{jpg,jpeg,png,gif}"GLOB_BRACE) as $premium_banner)
            
    unlink($premium_banner);
    Last edited: Aug 26, 2016
    vunguyenphs and Mark like this.
  2. Mark
    • Staff

    Mark Administrator Staff Member

    moved to core feature requests :)

    much better solution! this will be added to 1.6. Thanks VERY much for your contribution.
    cajkan likes this.
  3. Basti
    • Staff

    Basti Administrator Staff Member

    That stuff now changed in 1.6, though not in the way you suggested, ty for the suggestion though :) Was definitely better compared to the old.
    With the addition of gif to mp4 convert, we take whatever user has set in the database.
    One can have a different banner path now via plugins so hardcoded value isn't good anymore.
    Banner/mp4 paths now also include a timestamp to avoid cached images when a user reuploads a fresh image, so again checking {$username} will not work anymore, hence we take the database value


    Info why we decided against glob for anyone interested
    - As a sidenode, foreach isn't needed, as we only deal with a single file
    - On a low image count in the folder, glob seems slightly faster, but as image count increases into 1k+ file_exists seems to win by a huge margin. So we rather take the quicker method when many files are involved
    Last edited: Jan 19, 2019
    leonor and Mark like this.

Share This Page