-
Notifications
You must be signed in to change notification settings - Fork 5
Allow the results of one function to be the input for another function #96
Conversation
closes #94 |
Hi @aguilerapy, sorry but I don't like too much the approach you took here. The behaviour up to the selection of the next function is fine. But I don't think that staying on the same page is the best. I know that most functions don't have input options yet but we are planning to add some like #82 and especially #26. If we stay on the same page after selecting the function to execute next, we'll have to duplicate efforts by putting input options in different places or rewrite this one. When taking the results to another function you don't have to put them in the file upload box. Instead, for a first approach, you can show a simple list of the files generated by the previous function. In the future, we can think if it would be useful to allow the user to add more files or select the ones to use. |
What do you think about the solutions proposed for this PR? @jpmckinney |
I agree with @romifz's comment. I assume that we can put the output files in the session. Right now, many views have a |
Changes made to redirect to the page before processing the result. The option to add more files to process them together with the sent result is pending (if necessary). Also, to send more than one result it's necessary to make changes (for example, after adding split-package in #99) |
# Conflicts: # default/static/js/uploader.js # default/templates/default/base-uploader.html # default/templates/default/to-json.html # default/templates/default/to-spreadsheet.html # locale/es/LC_MESSAGES/django.po
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the solution and I like that the upload box is filled in with the name of the result file. Did you consider the case of multiple output files, e.g. from the upgrade function? When testing I see that only one file is taken.
I think that the server-side solution is a little bit more complex than it needs to be with the request.session['clear']
variable. I also think it's no good to clear the files after a single call, what would happen there if the user reloads the page?
Instead of making an ajax call to save the result contents into request.session['files']
I would do the following:
- Always save a
DataFile
for the last generated result within a session variable. - When a function receives the
destination
parameter in a GET request, the page displays the data of the last result as it is already doing now, but reading the data of the last result session variable. In the case of multiple result files, the session variable could be an array or the zip file info can be read here. - In the POST request, use a parameter to indicate that the last saved result should be used as input. Here the easiest is to write a decorator that will unzip the file and insert the results into
request.session['files']
before the method execution.
You should also avoid adding new buttons when they are not needed. The same 'Send' button should be used to execute the action. Having buttons that do very similar things and are in the same position, but appear alternately makes it difficult for another programmer to follow the code.
Also, why can't you send a file from compile
to to-spreadsheet
?
# Conflicts: # default/static/css/custom.css # default/templates/default/base.html
# Conflicts: # default/static/css/custom.css # default/static/js/uploader.js # default/templates/default/base-uploader.html # default/templates/default/to-json.html # default/templates/default/to-spreadsheet.html # default/views.py # locale/es/LC_MESSAGES/django.po
Since this is the last RP to merge based on #93, I'll add the help info panel. |
# Conflicts: # default/templates/default/to-spreadsheet.html # default/views.py # locale/es/LC_MESSAGES/django.po
# Conflicts: # default/templates/default/package-releases.html # default/util.py # default/views.py # locale/es/LC_MESSAGES/django.po
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for working on the inclusion tag. I've left one comment for a single improvement I think you can do. Otherwise, looks good to go.
We have implemented #14 (comment) here. There are 3 different behaviors for the
send
button:Right now, in the second step, it's only possible to go to 3 functions, due to the input/output file type:
None of these functions have input options or parameters yet, except Compile Releases with "Include Versioned Releases"
checkbox
, so option 1 is the one implemented.One possibility for option 1, is to show the parameters of the functions as we change the
selectbox
.