Ticket #8445 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

[ICAT] Removed unused try/catch from specific algorithms

Reported by: Jay Rainey Owned by: Jay Rainey
Priority: major Milestone: Release 3.1
Component: Framework Keywords: ICAT, ICAT4
Cc: Blocked By: #8192
Blocking: #6512 Tester: Samuel Jackson

Description (last modified by Jay Rainey) (diff)

When tested #6512 Roman noted:

Algorithms CatalogListInstruments, CatalogListInvestigationTypes, CatalogMyDataSearch catch all runtime errors and throw another one with the same message "Please login..." regardless of the cause of the caught exception and discarding its message. I think the exceptions need to be differentiated here, for example have a specialized LoginError.

This is true as these algorithms were previously used to test if the user was ​authenticated.

Once the inappropriate (logout, & search) are hidden until a user is logged in then this mechanism of authentication can be removed. Therefore, the try/catch used for this purpose from the following three algorithms will have to be removed:

  • CatalogListInstruments
  • CatalogListInvestigationTypes
  • CatalogMyDataSearch

The methods that make use of this logic will also have to be removed. They are: 

ICatHelper->validSession()
ICatHelper->openLoginDialog()

Change History

comment:1 Changed 7 years ago by Jay Rainey

  • Description modified (diff)

comment:2 Changed 7 years ago by Jay Rainey

  • Blocked By 8192 removed

comment:3 Changed 7 years ago by Jay Rainey

  • Blocked By 8192 added

comment:4 Changed 7 years ago by Jay Rainey

  • Status changed from new to inprogress

Removed unnecessary try/catch from catalog algors. Refs #8445.

  • The try/catch is no longer needed as we should not use these algorithms as a means of checking whether a user is logged in. Instead we have enabled/disabled the appropriate menus in another ticket.

Changeset: bfbd93932b7b090af3f8700ad23ffdb2e11171e0

comment:5 Changed 7 years ago by Jay Rainey

Removed methods that are no longer needed. Refs #8445.

Changeset: d87549d6c6f2c335765766c6bb1a5ecfa467620d

comment:6 Changed 7 years ago by Jay Rainey

Merge branch 'feature/8445_icat_refactor_algorithms' into develop. Refs #8445.

Conflicts:

  • Code/Mantid/MantidQt/MantidWidgets/inc/MantidQtMantidWidgets/CatalogHelper.h

Changeset: 13e0a14903355cee374c854dbc7473d5f38ca76d

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

comment:7 Changed 7 years ago by Jay Rainey

Tidy algorithms. Refs #8445.

  • Used auto when possible for cleaner code.
  • As most of the algorithms only make use of the catalog to invoke a specific method I now do this directly rather than in two steps.

Changeset: 870e791d8f4777e6b269faf6674df82e71556b2c

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

comment:8 Changed 7 years ago by Jay Rainey

Merge branch 'feature/8445_icat_refactor_algorithms' into develop. Refs #8445.

Conflicts:

  • Code/Mantid/Framework/ICat/src/CatalogSearch.cpp

Changeset: 3c37d84e5c669dd7b3e82fbde2249d09a66d02e2

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

comment:9 Changed 7 years ago by Jay Rainey

This ticket should only be tested once #8192 is tested.

To test

  1. Log into the catalog within Mantid ensuring that my code changes did not break any functionality.
  2. Perform a code review ensuring the changes I made are sensible and correct.
Last edited 7 years ago by Jay Rainey (previous) (diff)

comment:10 Changed 7 years ago by Jay Rainey

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

comment:11 Changed 7 years ago by Samuel Jackson

  • Status changed from verify to verifying
  • Tester set to Samuel Jackson

comment:12 Changed 7 years ago by Owen Arnold

  • Blocking 6512 added

comment:13 Changed 7 years ago by Jay Rainey

Merge remote-tracking branch 'origin/master' into feature/8445_icat_refactor_algorithms. Refs #8445

Conflicts:

  • Code/Mantid/Framework/ICat/src/CatalogLogin.cpp
  • Code/Mantid/MantidQt/MantidWidgets/inc/MantidQtMantidWidgets/CatalogHelper.h

Changeset: e7ad498e1a7622f457e1a790daa16643db4d6e41

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

comment:14 Changed 7 years ago by Samuel Jackson

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/8445_icat_refactor_algorithms'

Full changeset: ​c99203692c50a2d1b5f8c000ffbedacc3664d799

comment:15 Changed 7 years ago by Jay Rainey

Merge branch 'feature/8445_icat_refactor_algorithms' into develop. Refs #8445.

Conflicts:

Code/Mantid/MantidQt/MantidWidgets/inc/MantidQtMantidWidgets/CatalogHelper.h

Changeset: 13e0a14903355cee374c854dbc7473d5f38ca76d

comment:16 Changed 7 years ago by Jay Rainey

Tidy algorithms. Refs #8445.

  • Used auto when possible for cleaner code.
  • As most of the algorithms only make use of the catalog to invoke a specific method I now do this directly rather than in two steps.

Changeset: 28157ce92e614754c742b558ddd8c27c4c1b9dd7

comment:17 Changed 7 years ago by Jay Rainey

Merge branch 'feature/8445_icat_refactor_algorithms' into develop. Refs #8445.

Conflicts:

  • Code/Mantid/Framework/ICat/src/CatalogSearch.cpp

Changeset: 3c37d84e5c669dd7b3e82fbde2249d09a66d02e2

comment:18 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​9289

Note: See TracTickets for help on using tickets.