TechnicSolder icon indicating copy to clipboard operation
TechnicSolder copied to clipboard

Added the ability to clone modpacks entirely.

Open EntranceJew opened this issue 8 years ago • 2 comments

Addresses #71

  • Added a link in the sidebar to clone a modpack.
  • Cloning to an existing modpack will overwrite it with the source's packs.
  • Cloning to a nonexistent pack will create it.

I'm opening this up to talk about some of the concerns mentioned here and how they should be handled:

  • There are no permissions associated with cloning.
  • Destination list is loaded via JS therefore the modpacks have to be public even if the user has access to them. (Am I missing something here?)
  • No modpack permissions are considered before handling cloning to/from anything.

EntranceJew avatar Sep 28 '15 17:09 EntranceJew

Solid implementation! A couple things to note:

  • You should verify/sanitize the inputs before performing a Eloquent lookup. For Example:
$source = Modpack::where('slug', '=', Input::get('source'))->first();
$destination = Modpack::where('slug', '=', Input::get('destination'))->first();

vs.

$rules = array(
            'source' => 'required',
            'destination' => 'required'
         );

$messages = array(
            'source_required' => 'You must enter a modpack slug.',
            'destination_required' => 'You must enter a modpack slug'
         );

$validation = Validator::make(Input::all(), $rules, $messages);
    if ($validation->fails())
        return Redirect::to('modpack/create')->withErrors($validation->messages());

$source = Modpack::where('slug', '=', Input::get('source'))->first();
$destination = Modpack::where('slug', '=', Input::get('destination'))->first();

Yes its more code, but it prevents issues.

  • As for destination JS functions and loading from the API, thats a bit more tricky. Ideally instead of doing an AJAX call, you should populate both dropdowns with all modpacks on page load. There is no real need to update the destinations dropdown against the API, everytime the selected source modpack changes.

I'll work on implementing the permissions. If you could add some unit tests as well, that would be awesome! :+1:

GenPage avatar Sep 29 '15 03:09 GenPage

Sorry about necro-ing an old uncommitted commit. It fixes something I've been desperately wanting in Solder with the cloning, however unless I'm missing something it doesn't actually clone the mods associated with the builds of the source pack. I've got the new pack and builds, but no mod contents post-clone. Is there a permission issues I'm hitting? I'm not sure this is acting as intended, since the whole point of cloning it seems would be to migrate the mods as well.

Any ideas?

solder-1

silent-mobious avatar Apr 10 '16 18:04 silent-mobious