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

Migration guide? #9

Open
austenc opened this issue Mar 8, 2018 · 8 comments
Open

Migration guide? #9

austenc opened this issue Mar 8, 2018 · 8 comments

Comments

@austenc
Copy link
Contributor

austenc commented Mar 8, 2018

Hey @czim I'm interested in using this package instead of Stapler, but we already use Stapler (via laravel-stapler) in our project. I saw the list of differences in the readme, but was wondering if there was a more robust migration guide?

If not, is there anything else I should be aware of when attempting the transition? Is the rest of the API mostly the same as stapler's?

This looks far more compatible (sick of battling this every upgrade!), so I'd love to give it a shot! Thanks!

@austenc
Copy link
Contributor Author

austenc commented Mar 8, 2018

Additionally I am looking at my codebase and wondering if there is an equivalent to STAPLER_NULL or if with this package you can just use null?

Edit: Saw this line in the configuration docs, perhaps it should also be a callout in the main readme?

To make it easier to switch between Stapler and Paperclip, Stapler configurations are interpreted and normalized, so they don't need to be rewritten.

@czim
Copy link
Owner

czim commented Mar 8, 2018

There isn't a 1-to-1 migration guide at the moment. I'm swamped right now, and would greatly appreciate help in that regard. I completely agree that the documentation could do with a major overhaul to help (ex-)Stapler users.

  • The Paperclip equivalent for STAPLER_NULL is Czim\Paperclip\Attachment\Attachment::NULL_ATTACHMENT.
  • One important thing to test keep in mind and test carefully, is the configuration of the Laravel storage driver (Default driver handle is 'paperclip'). Stapler was a bit weird and inconsistent with the way local vs. S3 file storage was handled, for instance; in Paperclip this inconsistency is gone, because of the Laravel storage dependency.
  • Another thing to keep a close eye on, is the path.base-path configuration option. This is very similar to Stapler's filesystem.url and s3.path configuration options, but I've had reports that there are notable differences in the way path interpolation is performed. Not something I expect you'll have any issues with if you are/were using default values, but good to know if you had customized relative storage paths.

@austenc
Copy link
Contributor Author

austenc commented Mar 8, 2018

Hey, thanks for the extra info, that helps immensely. A few questions regarding your last two points:

  • The storage driver setup. My app was just using local storage for this, no s3 or anything extra / complex. Do I need to explicitly set the paperclip driver now, is that what you're saying? I ran into an error (screenshot below) that seems to be related to this.

  • As far as the stapler filesystem.url comment goes, is this akin to their config setting for determining the url of the uploaded asset? If so I'll have to update the lines I have like this in my application right?

        $this->hasAttachedFile('media', [
            'url' => '/system/:class_name/:id_partition/:filename'
        ]);

image

@austenc
Copy link
Contributor Author

austenc commented Mar 8, 2018

Another thing I'm wondering about is database migrations / tables. Do I need to update my existing stapler fields or add any?

@czim
Copy link
Owner

czim commented Mar 8, 2018

The driver error is indeed caused by the missing configured filesystem driver entry. You can configure paperclip to use an existing filesystem driver, or configure the filesystems to provide the default paperclip driver; either way works.

As for the url setting: correct. That's the behaviour that should be well tested depending on the parameters used.

The same table migrations are required, yes. You can add an additional field, which is documented in the readme, but it is not required. There is no paperclip:fasten command, which I considered overkill when it is so easy to write migrations in Laravel, though.

@rbruhn
Copy link

rbruhn commented Apr 19, 2018

@austenc Hey... finally had time to come check this out and replace laravel-stapler. Did you figure out the config settings? In the paperclip config, I tried 'local' and 'public' for the disks and I get back and error:

LaravelStorage.php
"trim() expects parameter 1 to be string, object given"

Never mind. I see that the the lines needed to be

// The Laravel storage disk to use.
'disk' => 'public', //'paperclip',

// Per disk, the base URL where attachments are stored at
'base-urls' => [
      'public' => config('app.url') . '/paperclip',
 ],

I'm figuring out the changes so will post if I run into other issues.
Another edit:
So far, this is what I have to match laravel stapler. In the paperclip config, I have these settings:

/*
|--------------------------------------------------------------------------
| Storage
|--------------------------------------------------------------------------
|
| Settings for handling storage of uploaded files.
|
*/

'storage' => [

    // The Laravel storage disk to use.
    'disk' => 'public',

    // Per disk, the base URL where attachments are stored at
    'base-urls' => [
        'public' => config('app.url'),
    ],
],

/*
|--------------------------------------------------------------------------
| Path
|--------------------------------------------------------------------------
|
| The storage path that uploaded files and variants for models are placed in.
|
*/

'path' => [

    // The class that generates the paths
    'interpolator' => \Czim\Paperclip\Path\Interpolator::class,

    // The base path that the interpolator should use
   // ':class/:id_partition/:attribute',
    'base-path' => '/files/:class/:attachment/:id_partition',
],

I'm not sure what the :attribute is doing, but it seemed to add "/document/" path right after the id_partition. This generates a url like below that matches my original laravel-stapler paths. I will do some more testing for thumbnails, various sized images, and post an edit.

"https://biospex.test/files/App/Models/Resource/documents/000/000/001/original/1-SERNEC_ExpeditionCreationProtocol_v1_r.pdf"

Edit: Something I should mention in case someone uses the above. The files are stored in 'storage' above my app directory. I use Envoyer with symlinks so files aren't wiped out on deployment.
storage -> ../storage
public/files -> ../storage/app/public/files

@austenc
Copy link
Contributor Author

austenc commented Apr 30, 2018

@rbruhn I'm finally getting around to doing this upgrade in our app that uses stapler. It's a beast of an app so it may take a little while, but thank you in advance very much for your comments! Were you able to transition completely to this package successfully?

@rbruhn
Copy link

rbruhn commented May 1, 2018

Ya, it's done using the method above. I was able to match the URLS so no need to move images or anything. I didn't put anything in my models regarding urls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants