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

Script upload #126

Merged
merged 13 commits into from
Feb 26, 2020
Merged

Script upload #126

merged 13 commits into from
Feb 26, 2020

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Feb 12, 2020

This PR allows webclient to upload official scripts using a UI similar to Insight's.

Screenshot 2020-02-12 at 13 17 13

Screenshot 2020-02-12 at 13 19 07

To test:

  • Login as Admin. Currently any Admin will see the "Upload Script" option in scripts menu. Do I need to check privileges for Light Admins?
  • Click the "Upload Script" option - launches dialog. Can choose local file to upload. No validation of file extension yet.
  • Choose a script path from the options. Also try to type one in.
  • The message should tell you whether you are uploading or replacing the script. Try both.
  • On submit, the script should upload and you'll see a success dialog.
  • On click "OK" the window should close and showing script menu in webclient should include your script.

@will-moore will-moore changed the title Support for Admins to upload scripts script upload Feb 12, 2020
@mtbc
Copy link
Member

mtbc commented Feb 18, 2020

Yeah, there are relevant WriteScriptRepo and DeleteScriptRepo privileges.

@jburel
Copy link
Member

jburel commented Feb 18, 2020

Upload the script via the UI under omero/figure_scripts as indicated in the README of figure.

# Create a dict we can use for scalebar unit conversions
unit_symbols = {}
unit_names = [
    "PICOMETER", "ANGSTROM", "NANOMETER", "MICROMETER", "MILLIMETER",
    "CENTIMETER", "METER", "KILOMETER", "MEGAMETER"
]
for name in unit_names:
    klass = getattr(UnitsLength, name)
    unit = LengthI(1, klass)
    to_microns = LengthI(unit, UnitsLength.MICROMETER)
    unit_symbols[name] = {
        'symbol': unit.getSymbol(),
        'microns': to_microns.getValue()
    }


This is correct. This is the new content of the script

@jburel
Copy link
Member

jburel commented Feb 18, 2020

Having feedback during the upload will be great. a wheel spinning or such like
Currently no feedback only a greyed out button

@jburel
Copy link
Member

jburel commented Feb 18, 2020

Tested the "replace script" option. This works too

@will-moore
Copy link
Member Author

@jburel Improved the UI when uploading. Show spinner and disable form inputs.

@dominikl
Copy link
Member

That's great, that's what people were asking for at the Edinburgh workshop too.

@jburel
Copy link
Member

jburel commented Feb 19, 2020

We will probably need to offer the ability to move scripts around
if for example it is put in the wrong location, easy mistake
or even remove a script

@jburel
Copy link
Member

jburel commented Feb 19, 2020

The text field where the location of the folder is added to is not disabled

@jburel
Copy link
Member

jburel commented Feb 19, 2020

Choose folder is not disabled

@will-moore will-moore mentioned this pull request Feb 19, 2020
@jburel
Copy link
Member

jburel commented Feb 20, 2020

Cancelling during the upload does not actually cancel it so this is misleading

@jburel
Copy link
Member

jburel commented Feb 20, 2020

Rename the button to close instead of cancel

@will-moore
Copy link
Member Author

That last commit allows you to see any ValidationException (e.g. Syntax Error in the script).

Screenshot 2020-02-21 at 15 51 06

@joshmoore
Copy link
Member

./omeroweb/webclient/views.py:4525:57: E128 continuation line under-indented for visual indent
2031./omeroweb/webclient/views.py:4531:5: E303 too many blank lines (2)

@jburel
Copy link
Member

jburel commented Feb 21, 2020

I tried to upload an invalid file (pdf). This was correctly not allowed. An error message pops up
but the upload button stays greyed out with the message uploading and the spinner is there do.
I have to close and start again

@will-moore
Copy link
Member Author

If user chooses non .py file (e.g. pdf) they will see:

Screenshot 2020-02-24 at 15 43 33

@jburel
Copy link
Member

jburel commented Feb 25, 2020

Restriction is now in place
If the file is not a valid script, an error is returned.
The dialog is closed when ok is pressed. This is a bit abrupt.
Probably good enough for a first round

@jburel jburel merged commit dfad916 into ome:master Feb 26, 2020
@jburel jburel changed the title script upload Script upload Feb 26, 2020
@jburel
Copy link
Member

jburel commented Feb 26, 2020

Tested with light-admin with upload script on: works fine
Tested with light-admin with upload script off: The user cannot upload, It will be better to have the option hidden in that case

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

Successfully merging this pull request may close these issues.

None yet

5 participants