Ticket #8537 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

[ICAT] Publishing GUI

Reported by: Jay Rainey Owned by: Jay Rainey
Priority: major Milestone: Release 3.1
Component: GUI Keywords: ICAT4,publish
Cc: Blocked By: #7636
Blocking: #8694, #8712, #8714 Tester: Peter Parker

Description (last modified by Jay Rainey) (diff)

Having spoke with Nick and Martin we have opted to create a new interface dialog box in order to replace the InvestigationNumber box with a drop-down menu that holds the investigation numbers of the investigations that the user is an investigator on. This will enable the user to easily select (without having to recall) the investigation that they want to publish files to.

As such, this ticket will involve:

  1. Create a new dialog ui using QTDesigner (CatalogPublishDialog.ui).
  2. Create a class (CatalogPublishDialog.cpp) that will use the ui file, and will populate the InvestigationNumber drop-down box with the user's investigation numbers (obtained via my data).
  3. Create a menu item in the catalog menu named "Publish". (Only display this if the user is logged in).
  4. Link the new publishing interface to the menu item.

Change History

comment:1 Changed 7 years ago by Jay Rainey

  • Blocking 8538 added

comment:1 Changed 7 years ago by Jay Rainey

  • Keywords ICAT4,publish added
  • Blocking 8538 removed

comment:2 Changed 7 years ago by Jay Rainey

  • Description modified (diff)

comment:3 Changed 7 years ago by Jay Rainey

  • Description modified (diff)

comment:4 Changed 7 years ago by Jay Rainey

  • Description modified (diff)

comment:6 Changed 7 years ago by Jay Rainey

  • Status changed from new to inprogress

Added base publishing dialog class. Refs #8537.

Changeset: d57adad6a0d0e64d6539c6896eb453a579a00f06

comment:7 Changed 7 years ago by Jay Rainey

Added publishing dialog ui file. Refs #8537.

Changeset: ec35c2f37dab48b9e48cad049198d575ec8c3130

comment:8 Changed 7 years ago by Jay Rainey

Override generated form layout. Refs #8537.

Changeset: 138897463fd9d9cb04b914ce17f02670a1175451

comment:9 Changed 7 years ago by Jay Rainey

Populate combo-box with user investigation IDs. Refs #8537.

Changeset: 9858b2f1675bf631bf9ebfcd3153bfd00c8ea772

comment:10 Changed 7 years ago by Jay Rainey

Added browser file selection to GUI. Refs #8537.

Changeset: 1d2f3b173d23a5450fd139daec1eec249425f85f

comment:11 Changed 7 years ago by Jay Rainey

Added custom publish dialog to catalog menu. Refs #8537.

Changeset: 5652a17164daf57ee7033bad82e5f4d4eea7607e

comment:12 Changed 7 years ago by Jay Rainey

Updated tooltip message. Refs #8537.

Changeset: a321374f55f5db01e660a6c3eba38a2f0cbb51b9

comment:13 Changed 7 years ago by Jay Rainey

Minor comment improvements. Refs #8537.

Changeset: bf43a3ad12d949e2dd85afc9b78e7eafbca75af6

comment:14 Changed 7 years ago by Jay Rainey

Use investigationID NOT datafileName to obtain datasetID. Refs #8537.

  • Changed the query so that rather than searching for a dataset based on a datafile name we now use the investigationID (which is provided to the user in the GUI).
  • I also updated all related variable names in each catalog for readability.

Changeset: 91755080fc41d49cc52975e7476ca8cb09874c5e

comment:15 Changed 7 years ago by Jay Rainey

Simplify and tidied CatalogPublish algorithm. Refs #8537.

  • Use investigation number instead of datafile name for searching.
  • Removed unused method (extractFileName).

Changeset: 0072b97eb04a4c207e2356e3b85e64f4be5c8fff

comment:16 Changed 7 years ago by Jay Rainey

Forgot to add license/documentation to dialog. Refs #8537.

Changeset: eefb209f89166cb0d4a517d06d01dcbb54bbf357

comment:17 Changed 7 years ago by Jay Rainey

Improved algorithm input validation. Refs #8537.

  • Prevent invalid naming of the file being saved to the catalog.
  • Inform user we are using the filename or workspace name as the name of the file we are publishing. This occurs if the user has not provided a name for the file to be saved as.

Changeset: c1e9562819039ca1d9aff185a023c2b2659325b8

comment:18 Changed 7 years ago by Jay Rainey

Add optional message to GUI. Refs #8537.

Changeset: a7ebbc5e5573cf8839475d9d575c32aeede09afd

comment:19 Changed 7 years ago by Jay Rainey

Disable buttons if user cannot publish. Refs #8537.

  • If the user is not an investigator on any investigations then they cannot publish. As such, I have disabled the input fields and Run button on the GUI.

Changeset: d758d93559ab8854746603a8bd01cb720db3d971

comment:20 Changed 7 years ago by Jay Rainey

  • Status changed from inprogress to verify
  • Resolution set to fixed

Note

  • You can only publish data to investigations of which you are an investigator on.
  • This ticket needs to be tested on multiple platforms.
  • You can only test this ticket if you have results in "my data". (E.g. you are an investigator in investigations.)

Overview

This ticket dealt with creating a GUI for the publishing algorithm. Rather than creating a new GUI I overrode the default dialog box in order to populate the investigationNumber field with a drop-down that contains the investigation numbers of each investigation of which the user is an investigator.

To test

Change Facilities.xml

You will need to change the soapendpoint and externaldownload in Facilities.xml to:

  soapendpoint     -> "https://icatdev.isis.cclrc.ac.uk/ICATService/ICAT"
  externaldownload -> "https://isisoxfordvmsrv.isis.cclrc.ac.uk/ids/"

New publishing GUI

  1. Log into the catalog.
  2. A new item in the menu should appear named "Publish". Click it and the new catalog publish GUI will appear.
  3. From the new GUI select a file to upload, provide a name, and select an investigation where you want to upload the file to. There is a tooltip for each item in the combo-box to provide more details about the investigation, such as title and instrument name.
  4. Verify that the file has been uploaded by using the Search interface to find it in the archives.

comment:21 Changed 7 years ago by Peter Parker

  • Status changed from verify to reopened
  • Resolution fixed deleted

There are a few small issues to fix. I'll let you decide whether or not any of them should be in new tickets.

  • A user who hasn't logged into ICAT will see MantidPlot crash if they open up a CatalogPublish dialog. A suggestion you should feel free to ignore: in this case should the dialog fields actually be disabled, similarly to when a non-investigator user logs in?
  • The wiki page accessed by pressing the "?" button has "INSERT FULL DESCRIPTION HERE" written in the Description section.
  • Can we improve the check for a successful response HTTP status in line 140 of CatalogPublish.cpp? Rather than casting to a string and grepping for "20", we should probably just do a straight-forward check to see if the status is equal to one (or more?) of the actual Poco HTTPStatus enum values.
  • Whenever the following error occurs:

An error has occurred on the ICAT IDS server. A file with that name already exists or you do not have permissions to publish to that investigation.

the algorithm still outputs "CatalogPublish Successful" to the results log. In such a case, the algorithm should throw so that it isn't allowed to "finish".

  • The report log indicates that the "NameInCatalog" field has not been set, even if it has. A code inspection suggests that this is just a cosmetic problem. (Lines 79 and 84 of CatalogPublish.cpp).

comment:22 Changed 7 years ago by Jay Rainey

  • Status changed from reopened to inprogress

Correct catalogpublish subcategory. Refs #8537.

Changeset: daa2a9e6cf99a0a289a4e85ba29c8e97c69d27e1

comment:23 Changed 7 years ago by Jay Rainey

Update documentation for catalogpublish. Refs #8537.

Changeset: ecd43ce9d92efa9b7a47473e20daf66a0842009c

comment:24 Changed 7 years ago by Jay Rainey

Log message if nameInCatalog has not been set. Refs #8537.

Changeset: b994c4c9f8f55d4f6dc27e9dd9d69d55acd6992b

comment:25 Changed 7 years ago by Jay Rainey

Cancel algorithm if it fails. Refs #8537.

Changeset: 81cb7acef145774ea3fb213c185f9b6454f85b38

comment:26 Changed 7 years ago by Jay Rainey

Prevent hang, and improve optional message. Refs #8537.

  • The dialog will not appear correctly if the user is not logged in rather than hanging. This was due to making a request to ICAT within a session ID as "myData" is called within the CatalogPublishDialog.

Changeset: 76240e4e4c9d88740991e0f3d042b869d1604363

comment:27 Changed 7 years ago by Jay Rainey

Improved algorithm documentation. Refs #8537.

Changeset: 777f6bdc73e52bda0ce93ee7b557f2abc18b12c2

comment:28 Changed 7 years ago by Jay Rainey

  • Status changed from inprogress to verify
  • Resolution set to fixed

Replying to Peter Parker:

A user who hasn't logged into ICAT will see MantidPlot crash if they open up a CatalogPublish dialog. A suggestion you should feel free to ignore: in this case should the dialog fields actually be disabled, similarly to when a non-investigator user logs in?

This was due to attempting to perform a "my data" search with an empty session ID (e.g. user was not logged in). The crash no longer occurs. I have opted to disable the dialog fields similar to when a non-investigator user logs in, and updated the optional message. See: commit

The wiki page accessed by pressing the "?" button has "INSERT FULL DESCRIPTION HERE" written in the Description section.

I have written better documentation, which can be seen in comment:23 commit and was improved further in comment:27 commit.

Can we improve the check for a successful response HTTP status in line 140 of CatalogPublish.cpp? Rather than casting to a string and grepping for "20", we should probably just do a straight-forward check to see if the status is equal to one (or more?) of the actual Poco HTTPStatus enum values.

This will be addressed in a separate ticket as I am having a meeting with Tom tomorrow in regards to better error handling for ICAT/IDS REST service within Mantid.

The algorithm still outputs "CatalogPublish Successful" to the results log. In such a case, the algorithm should throw so that it isn't allowed to "finish".

As POCO has a socket bug (as noted in CatalogDownloadDatafiles here) that needs to be caught when performing HTTPS requests I was unable to throw an exception (as the I/O exception would be thrown instead) when the response was invalid.

Instead, I have opted to cancel the algorithm and log the message to the user. See this commit.

The report log indicates that the "NameInCatalog" field has not been set, even if it has. A code inspection suggests that this is just a cosmetic problem. (Lines 79 and 84 of CatalogPublish.cpp).

This was due to using short-hand IF statements (second element wasn't included in the IF). I have addressed this.

Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:29 Changed 7 years ago by Jay Rainey

  • Blocking 8694, 8712, 8714 added

comment:30 Changed 7 years ago by Peter Parker

  • Status changed from verify to verifying
  • Tester set to Peter Parker

comment:31 Changed 7 years ago by Peter Parker

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/8537_icat_publishing_gui'

Full changeset: 4f2821bf4724f669c82cf08d01a5781960b9f67e

comment:32 Changed 7 years ago by Peter Parker

Most recent changes look good, and work for me.

comment:33 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 9381

Note: See TracTickets for help on using tickets.