Ticket #8322 (closed: fixed)
[ICAT] Tidy algorithms
Reported by: | Jay Rainey | Owned by: | Jay Rainey |
---|---|---|---|
Priority: | minor | Milestone: | Release 3.1 |
Component: | Framework | Keywords: | ICAT,ICAT4,maintenance |
Cc: | Blocked By: | ||
Blocking: | #8230, #8244 | Tester: | Russell Taylor |
Description (last modified by Jay Rainey) (diff)
The catalog algorithms have repeated code throughout (error handling, and creating the algorithms). Several of the algorithms could be tidied up.
- Encapsulate repeated code in an AlgorithmHelper class.
- Tidy each algorithm to improve readability and code-reuse.
Change History
comment:2 Changed 7 years ago by Jay Rainey
- Status changed from new to inprogress
Added algorithm helper class. Refs #8322.
- This class reduces the code across algorithms.
Changeset: 8f83ed0f1f163515ba03714b94200f8d0ddb50f5
comment:3 Changed 7 years ago by Jay Rainey
Refactored CatalogSearch. Refs #8322.
- Removed unused attributes from CatalogSearchParam.
- Updated ICat3Helper.cpp to work correctly with new interface (e.g. when "my data" is checked).
- Updated documentation for algorithm.
Changeset: 54b3ea68da2d8507d68e86c83c4d5d857ae37b20
comment:4 Changed 7 years ago by Jay Rainey
Tidied CatalogSearchParam class. Refs #8322.
- Removed unnecessary comments in header. Made them more concise and consistent with other header files in Mantid.
- Removed RB number search as it is no longer used in the interface.
- Added investigator surname search back to CatalogSearch.
Changeset: f78d67913d3464ee403f0203363b79a80f28d3e6
comment:5 Changed 7 years ago by Jay Rainey
Tidied up CatalogMyDataSearch. Refs #8322.
- Removed unused headers and added call to helper class.
Changeset: f46abacb0d4e27364e2dd8f17bdb1d5fde01f558
comment:6 Changed 7 years ago by Jay Rainey
Refactored catalog logout algorithm. Refs #8322.
- Removed unimplemented method doLogout().
Changeset: 6048e15f8716a911203876c8c6921d474918dffe
comment:7 Changed 7 years ago by Jay Rainey
Tidied Catalog login algorithm. Refs #8322.
- Removed unnecessary catalog error method.
Changeset: 3807a3f8b8b87f815309f5f1819357ff7deac46d
comment:8 Changed 7 years ago by Jay Rainey
Updated CatalogListInstruments. Refs #8322.
Changeset: 98e4cee93ae32969d220f9f7c2f075adca0be023
comment:9 Changed 7 years ago by Jay Rainey
Made use of algorithm helper class. Refs #8322.
- Removed unused methods (filterLogFiles & isDataFile) and related properties.
Changeset: 590949376b2355fa96ea2c9bd225ad019699ffce
comment:10 Changed 7 years ago by Jay Rainey
Use algorithm helper class. Refs #8322.
- Updated documentation for each algorithm and tidied header files.
Changeset: 07747cc51c2624900d14e2846399475096911b97
comment:11 Changed 7 years ago by Jay Rainey
Updated CatalogDownloadDataFiles. Refs #8322.
- Added call to helper class and removed un-used headers.
- Removed using Poco namespaces as each are only called once or twice in the download method.
Changeset: 1026afbe1f742e401dadfda0f40fbdf0fa9660ed
comment:12 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
- Blocking 6512 removed
comment:15 Changed 7 years ago by Jay Rainey
Added datafileName & removed investigation abstract. Refs #8322.
- Previously, the functionality existed to search by datafile name, but was not implemented on the interface. I have re-added this functionality (removed it in a previous commit), and replaced the investigation abstract on the GUI with datafile name.
- I removed all references to investigation abstract search.
Changeset: fcfe0e8360d7c716b67771c7d787a4a53e9ca491
comment:16 Changed 7 years ago by Jay Rainey
I have moved code that was repeated across catalog algorithms to a separate class for re-usability, and tidied up each algorithm by removing unused methods and generally improving readability.
To test
- Open Mantid and login into the catalog with your federal ID.
- Ensure my changes did not break any functionality by performing searches and using all search input fields.
- As I have removed the functionality to search by investigation abstract, and added functionality to search by datafile name I would like this to be tested. (An example of a datafile name is: 62862).
As this ticket dealt with refactoring a code review of my changes would be appreciated.
comment:17 Changed 7 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:18 Changed 7 years ago by Martyn Gigg
- Status changed from verifying to reopened
- Resolution fixed deleted
There are merge conflicts that I can't see how to solve. Could you merge master to your branch & fix them there, please?
comment:19 Changed 7 years ago by Jay Rainey
- Status changed from reopened to inprogress
Merge remote-tracking branch 'origin/master' into feature/8322_icat_refactor_algorithms. Refs #8322.
Conflicts:
- Code/Mantid/Framework/ICat/inc/MantidICat/CatalogSearchParam.h
- Code/Mantid/Framework/ICat/src/CatalogSearchParam.cpp
Changeset: d6cce43c4207203b4750216f2e530ca321119e6e
comment:20 Changed 7 years ago by Jay Rainey
Merge branch 'feature/8322_icat_refactor_algorithms' into develop. Refs #8322.
Conflicts:
- Code/Mantid/Framework/ICat/src/CatalogSearchParam.cpp
Changeset: 40503b8cb9ece0c768dd900276c6842897917eb2
comment:21 Changed 7 years ago by Jay Rainey
- Status changed from inprogress to verify
- Resolution set to fixed
comment:22 Changed 7 years ago by Jay Rainey
comment:24 Changed 7 years ago by Russell Taylor
- Status changed from verify to verifying
- Tester changed from Martyn Gigg to Russell Taylor
comment:25 follow-up: ↓ 27 Changed 7 years ago by Russell Taylor
The code looks OK. I have found one bug, but it was present before so I'll pass this one.
- The bug is that if I try to load a .log file, it fails. More importantly, from that point on nothing works - loading a .raw file, initiating a new search... - they all just give an error.
- The search box appears in strange places (like the top-left of the screen). It should appear in the centre of the MantidPlot window. If I recall correctly, the way to do this is to make the ApplicationWindow the parent of the widget. This will also give the window the Mantid icon.
- The name search always returns nothing for me, and it seems unlikely that there should be no result for surnames like Smith, Brown, Taylor, Chapon....
comment:26 Changed 7 years ago by Russell Taylor
- Status changed from verifying to closed
Merge remote branch 'origin/feature/8322_icat_refactor_algorithms'
Full changeset: f16b168978e9ccf790e90e383e039c00db53ac99
comment:27 in reply to: ↑ 25 Changed 7 years ago by Jay Rainey
Replying to Russell Taylor:
- The bug is that if I try to load a .log file, it fails. More importantly, from that point on nothing works - loading a .raw file, initiating a new search... - they all just give an error.
This appears to only fail on OSX. I created a ticket for it - #8442.
- The search box appears in strange places (like the top-left of the screen). It should appear in the centre of the MantidPlot window. If I recall correctly, the way to do this is to make the ApplicationWindow the parent of the widget. This will also give the window the Mantid icon.
I will address this issue in #8380.
- The name search always returns nothing for me, and it seems unlikely that there should be no result for surnames like Smith, Brown, Taylor, Chapon....
This will be addressed when moving to the new query language in #8401.
comment:28 Changed 7 years ago by Jay Rainey
Merge branch 'feature/8322_icat_refactor_algorithms' into develop. Refs #8322.
Conflicts:
Code/Mantid/Framework/ICat/src/CatalogSearchParam.cpp
Changeset: 40503b8cb9ece0c768dd900276c6842897917eb2
comment:29 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9167