knownasilya/ember-plupload

Uploader options set on pluploader are overriden before upload

Closed this issue · 9 comments

I'm using uploader.setOption. to set custom property on pluploader instance in onInitOfUploader hook:

onInitOfUploader(pluploader) {
    pluploader.setOption('send_file_name', false)
}

Yet, when upload is done, filename is sent. After debugging I found, that whatever option is set on uploader, it's overridden by BeforeUpload listener in configureUpload function in upload-queue.js

  configureUpload(uploader, file) {
    file = this.findBy('id', file.id);
    // Reset settings for merging 
    uploader.settings = copy(get(this, 'settings'));
    merge(uploader.settings, file.settings);

    this.progressDidChange(uploader, file);
  },

I thought, that I could pass the option directly to file.upload(ur, settings), since those are merged in the next line, but file ignores all unknown settings returning preset keys only from settingsToConfig function.

  return {
    url: url,
    method: method,
    headers: headers,
    multipart: multipart,
    multipart_params: data,
    max_retries: maxRetries,
    chunk_size: chunkSize,
    file_data_name: fileKey
  };

Is there a way then to pass custom option to uploader?

Ahh, I could add this as an option. The other way to do it is to pass a config hash to the uploader in the template.

@tim-evans is this passing hash currently supported, or you are talking about possible solution? If it's supported, how exactly would that look?

Btw, is there a reason for resetting uploader settings? I would say, the init hook looks like a good place to customise the pluploader instance.

Pretty sure if you provide config as an attribute: https://github.com/tim-evans/ember-plupload/blob/master/addon/components/pl-uploader.js#L85

It will properly get merged into the upload queue.

The reason why the uploader settings are reset is to provide the nice file.upload() API. Plupload requires you to configure all of that up front instead of configuring it at upload time. So, it's a hack to get that to work.

Oh, right. Technically possible, but overriding config sounds like a hack if you ask me. Also, in order to provide one option, you'd need to provide all others, not to mention, that this actually makes other params (which are used to build that config in component) moot.

On general, Ember level, I consider the sole possibility to override internal component functions and properties, by passing them from template, as breaking the encapsulation. I do not know why Ember components are designed this way. I believe params should be on a completely different namespace than component internals and accessed eg. via this.params.

Speaking of nice file.upload() API. How about passing the additional option to file.upload? What would need to be changed is the settingsToConfig to allow more or any props instead of limited set (https://github.com/tim-evans/ember-plupload/blob/master/addon/system/file.js#L60) That could be more in line with the per-file config approach, you have mentioned.

Btw, I understand the reset tries to simulate a separated uploader environments for each file using single instance, do you also restore configs on any async action from file? How those are handled, can potentially depend on current uploader config as well.

@gustaff-weldon I'm currently running into the exact same issue of wanting to set send_file_name: false.

Can you kindly advise what you did to ensure the setting does not get overriden ?

I did:

onInitOfUploader(pluploader) {
    pluploader.setOption('send_file_name', false)
}

and:

{{#pl-uploader onInitOfUploader=(action "onInitOfUploader")}}
{{/pl-uploader

See #63 (comment) for a solution. Setting an option using setOption will be overriden due to the addon copying the config prior to this function.

For some backstory, the config is stored and copied on the service so it can be merged with the new config passed into upload. This is difficult to time correctly, and to get the API working as I wanted it to, this was the consolation.

@tim-evans thank you so much for the reply!
Sorry in advance for the terrible followup question (fairly new to ember-js / templates), but can you please advise how I can pass in a parameter like send-file_name = false via the config attribute you mentioned in #63 comment?

Would it be something on the lines of:

{{#pl-uploader send_file_name=false }}
{{/pl-uploader}}

I noticed this particular attribute is not searched/parsed for in the config function you linked to.

Well, it looks like this will be a bigger pain not to patch, so I've added it.

See 08be589

This is in the most recent release 🎉

Thank you @tim-evans ! I really appreciate the fix.
Using the component in my project right now and love it!