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

New feature: API method to get all uploaded files for a given token #642

Merged
merged 1 commit into from Feb 23, 2017
Merged

New feature: API method to get all uploaded files for a given token #642

merged 1 commit into from Feb 23, 2017

Conversation

@Shnoulle
Copy link
Collaborator

One file only here ?

If you need all files from token : must be zipped
If you need one file : must restrict to filename ?

Else : see https://manual.limesurvey.org/Standard_for_Git_commit_messages

@ghost
Copy link
Author

ghost commented Feb 23, 2017

One file only here ?

Sorry, I don't understand this question.

If you need all files from token : must be zipped

Should it return base64 encoded zip files? It should be smaller. Most likely, you would need to unzip on the client side anyway for further processing/storing on disk.

If you need one file : must restrict to filename ?

It returns the uploads referenced by the name of the file on disk. Like:

 [uploads] => Array
 (
   [fu_wdmygs38jjueuti] => Array
   (
      [meta] => Array
      (
         [title] => My CV
         [comment] => no comment
         [size] => 469.188
         [name] => CV.pdf
         [filename] => fu_wdmygs38jjueuti
         [ext] => pdf
      )

      [content] => JVBERx0xLjQKJaqrrK0KNCAwIG9iago8PAovVGl0bGUgKHVua25vd24pCi9DcmVhdG9yIChBcGFjaGUg
....
etc, etc

I could give it an extra optional parameter to specify the file you'd like to get.

I'm sorry for not paying attention to the commit message guide lines. I don't think I can change the commit message for this commit anymore...

Bests,

Bram

@Shnoulle
Copy link
Collaborator

Oups, yes OK : return a json with base64 encode file content. No need to zip.

About commit message : http://stackoverflow.com/a/179147/2239406

git commit --amend -m "New feature : ....."
git push  brammeleman rpc-get-uploaded-files-feature -f

And now , my Point Of View about this feature : can be in a plugin ;)

@olleharstedt
Copy link
Contributor

@Shnoulle Why would it be in a plugin?

My view: Every API command should be its own command class, instead of add methods to the existent class indefinitely. But that would take some refactoring.

@ghost
Copy link
Author

ghost commented Feb 23, 2017

@Shnoulle: Thanks for the git hint! It worked!

And now , my Point Of View about this feature : can be in a plugin ;)

To my opinion, it should be part of the LS remote API. It does not make sense to be able to retrieve the survey responses, but not the uploads (which are also responses). I know you can construct an url to the file and download it, but only if the uploads directory is in the webservers document root. Which is not an option in our setup (confidential documents are being uploaded).

@ghost ghost changed the title added method to get all uploaded files for a given token New feature: API method to get all uploaded files for a given token Feb 23, 2017
@Shnoulle
Copy link
Collaborator

Why in plugin : default ls behaviour when donwload file : send a zip to browser , then default behaviour from RPC : send a zip :).

@olleharstedt : Every API command should be its own command class +1 and adding API command can be done via extension/plugin

@Shnoulle
Copy link
Collaborator

And i say : can not must

I fact : i think ALL RPC command must be extension, except the connexion key.

@olleharstedt
Copy link
Contributor

Yes, adding a new command should as easy as adding a new class to a command namespace, from plugin or where ever.

@ghost
Copy link
Author

ghost commented Feb 23, 2017

@Shnoulle I get the point. At this point, I'm not too interested in a LS architecture discussion ;-)

@ghost
Copy link
Author

ghost commented Feb 23, 2017

@Shnoulle

Why in plugin : default ls behaviour when donwload file : send a zip to browser , then default behaviour from RPC : send a zip :).

If you browse the responses and click an uploaded document, you'll get the document as uploaded. Not wrapped in a zip.

@Shnoulle
Copy link
Collaborator

If you browse the responses and click an uploaded document, you'll get the document as uploaded. Not wrapped in a zip.

Not for whole reponses (1 response line) if i don't make error :)

@Shnoulle
Copy link
Collaborator

Shnoulle commented Feb 23, 2017

@Shnoulle I get the point. At this point, I'm not too interested in a LS architecture discussion ;-)

Then : maybe best is to do a plugin : http://extensions.sondages.pro/development-and-example/extend-remotecontrol-api/

Because next time we update LS architecture, we must update your code if it was integrated in LS

@LouisGac
Copy link
Contributor

this PR seems good to me. I don't understand all this debate. We must encourage people to do PR. This PR make the RPC better, bringing a functionality that should be core: RPC should provide a way to get relevant datas for all question types, even for file upload.

@c-schmitz
Copy link
Contributor

+1

@LouisGac LouisGac merged commit c8878f0 into LimeSurvey:master Feb 23, 2017
@ghost
Copy link
Author

ghost commented Feb 23, 2017

thanks!

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