Ticket #10392 (closed: fixed)
New Reflectometry GUI needs search pane.
Reported by: | Owen Arnold | Owned by: | Harry Jeffery |
---|---|---|---|
Priority: | major | Milestone: | Release 3.3 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #10540 | Tester: | Owen Arnold |
Description
This has been implemented in the old reflectometry gui, and roughly we should duplicate the behaviour. There are a few things I would like to see in addition.
- The consumption of the ICAT search tools should be abstracted if possible. We have already had one occasion to switch the search tools from xml journal to icat, one could imagine this happening again.
- The functionality should continue to provide good and useful feedback such as allowing the user to login to ICAT in situ.
- There are currently as set of rules for transfer and grouping runs into rows and columns (ask Owen for details, but the code for this is in the old gui). This transfer logic should be encapsulated, the type should take in a TableWorkspace or some equivalent to perform the transfer on so that we can test the search functionality without the network. This transfer functionality would have access to the presented table workspace so that it could populate it. Also this type of transfer has been implemented for Max by Max, I'd like to have the ability to switch to different transfer strategies, as Max's one may not be universally liked.
Change History
comment:2 Changed 6 years ago by Owen Arnold
Would also be VERY nice to have a preview/accept of the transfer maybe highlight the changes in some colour prior to agreeing. The old GUI can't do this, and so once the transfer has been pressed the table needs to be rolled-back.
comment:4 Changed 6 years ago by Harry Jeffery
Refs #10392 Add getSearchString accessor
Changeset: 224b5f0fa43717d9796c9699b9de03eb15da49a8
comment:5 Changed 6 years ago by Harry Jeffery
Refs #10392 Add placeholder search action
Changeset: 96f710294298742de3e93e638221121d0df4c21b
comment:6 Changed 6 years ago by Harry Jeffery
Refs #10392 Provide IReflSearcher interface for searching
Changeset: 09366e46af47277316523277db2bee0900845c0c
comment:7 Changed 6 years ago by Harry Jeffery
Refs #10392 Show search results in table
Changeset: 1e16994e1a274793f2f28222b8711ab7d88c6c12
comment:8 Changed 6 years ago by Harry Jeffery
Refs #10392 Clean up Catalog search results
Changeset: 76482adb784efa95ae4b697d5c3e1bea847a55ed
comment:9 Changed 6 years ago by Harry Jeffery
Refs #10392 Don't use nullptr
RHEL6 and OSX don't seem to like it.
Changeset: e96627d53811e22fb6a9f6da8a7a69670966e24d
comment:10 Changed 6 years ago by Harry Jeffery
Refs #10392 Discard incorrect comments
Changeset: b27e98e8a2e72ded979dcf7ee2c2ccd7271a59fc
comment:11 Changed 6 years ago by Harry Jeffery
Refs #10392 Sort runs list in ReflSearchModel
This commit fundamentally changes the behaviour of ReflSearchModel. Instead of merely wrapping a ITableWorkspace, it copies the data it cares about out of the workspace and then forgets about it. The main reason for doing this is the lack of sorting functionality in the TableWorkspace class. Weighing things up, the simplest way forward is simply to look after the data we care about ourselves and manage it internally.
Changeset: 44ea276fe3cc17e67a9eeef158f1d2469291b661
comment:12 Changed 6 years ago by Harry Jeffery
Refs #10392 Add unit test for ReflSearchModel
This introduces the groundwork we'll be using to test transferring runs.
Changeset: 5e909f8f4390a3f1da772ac68ca6527572c2252d
comment:13 Changed 6 years ago by Harry Jeffery
Refs #10392 Add placeholder "Transfer" action
Changeset: 405252fa11b703f9abc792328964f94370dcaa3c
comment:14 Changed 6 years ago by Harry Jeffery
Refs #10392 Add getSelectedSearchRows accessor
Changeset: 10360b9a7c3f34b77080fa8d5642bbb6d9186397
comment:15 Changed 6 years ago by Harry Jeffery
Refs #10392 Implement transfer
Changeset: 93e58af63e7204d57df20da5663b4423428f9b52
comment:16 Changed 6 years ago by Harry Jeffery
Refs #10392 Make theta extraction during transfer optional
Changeset: 7bbb2ba96fbf3ef1c8e3c0692295b121c3dd3c32
comment:17 Changed 6 years ago by Harry Jeffery
Refs #10392 Make run grouping optional
Changeset: ba13827ccee629ef8a6dbc2e9fa86f76d079d391
comment:18 Changed 6 years ago by Harry Jeffery
Revert "Refs #10392 Add unit test for ReflSearchModel"
This reverts commit 5e909f8f4390a3f1da772ac68ca6527572c2252d.
Changeset: 6bab553aeb649b44fdbb99d0fcaeddd7f5d656b6
comment:19 Changed 6 years ago by Harry Jeffery
Refs #10392 Add context menu to search results table
Changeset: d2dca9ec0672c8c3d15cd755d97ab481bfc45fb9
comment:20 Changed 6 years ago by Harry Jeffery
Refs #10392 Move existing transfer code into strategy pattern
Changeset: f42b2687d00078445ed19e7abc82f877cb46e61b
comment:21 Changed 6 years ago by Harry Jeffery
Refs #10392 Remove unused options
Changeset: 9cd101c1971f73c16a21ff34eea82f298340fca4
comment:22 Changed 6 years ago by Harry Jeffery
Refs #10392 Rewrite LegacyTransferStrategy to emulate the old Refl UI
Changeset: c42f43bf45e19c38d571d00b7b760d0ebd84c816
comment:23 Changed 6 years ago by Harry Jeffery
Refs #10392 Sort the output of ReflLegacyTransferStrategy
Changeset: 114942f89d29fa173e617b9c2cf431becac503b9
comment:24 Changed 6 years ago by Harry Jeffery
Refs #10392 Unit test ReflLegacyTransferStrategy
Changeset: 1b4be75f053cd794aaae7471835571a32ecf672e
comment:25 Changed 6 years ago by Harry Jeffery
Refs #10392 Fix the windows build
Changeset: b2c59a638ad864326804f2a6aece6e03b3eb63e3
comment:26 Changed 6 years ago by Harry Jeffery
Refs #10392 Add missing include
Changeset: 13a8e259eb8e1f2818de260152eb1195a8cc1853
comment:27 Changed 6 years ago by Harry Jeffery
Refs #10392 Fix link error on windows build
Changeset: 4dd74bcbbc6ad4e26e739410b7c4a9e928095cdc
comment:28 Changed 6 years ago by Harry Jeffery
Refs #10392 Tidy up ReflLegacyTransferStrategy
Hopefully this will also fix the failing test on rhel6. The logic seems perfectly fine when debugged locally on ubuntu.
Changeset: ef370e305fb0d5c748057c00f6b8237cc960c6d0
comment:29 Changed 6 years ago by Harry Jeffery
Refs #10392 Fix substr exception on RHEL6
What was the cause of this mysterious bug? An older version of boost::regex. It was matching successfully but not bothering to fill out the match object, so the position parameter being passed to string::substr was just uninitialised stack. Telling boost to reset branching between each pattern solves this problem.
Changeset: 45537791aa9762e549c045a43941ffbc2f12cf44
comment:30 Changed 6 years ago by Harry Jeffery
Refs #10392 Don't attempt to search for nothing
Changeset: 237d6b249e0b8341b5290c1ae64b282bca99b0f3
comment:31 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
Testing
- Login to a catalog
- Open the new Reflectometry UI
- Search for some runs
- INTER with 1300014 brings up plenty of results
- Try transferring some runs across to the processing table
- Transfers should obey these rules:
- Runs with the same description (regardless of theta) should be placed in the same group
- Runs with the exact same description, including theta, should be placed in the same row, joined with '+'
- If theta is given in the run description, it should always be prefilled when transferred to the processing table
- Transfers should obey these rules:
- Verify that the right-click menu also works for transferring runs
- Verify all CustomInterfaces tests are passing
comment:32 Changed 6 years ago by Harry Jeffery
- Status changed from verify to reopened
- Resolution fixed deleted
comment:34 Changed 6 years ago by Harry Jeffery
Refs #10392 Add "Whats This?" tips to search interface
Changeset: 8e60dde2544086b1aecc55b26f78c432762c01d0
comment:35 Changed 6 years ago by Harry Jeffery
- Status changed from inprogress to verify
- Resolution set to fixed
comment:36 Changed 6 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
comment:37 Changed 6 years ago by Owen Arnold
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/10392_refl_ui_search'
Full changeset: 7d1967b4fe104860e4ae046b8550ff06ec27ab3f
comment:38 Changed 6 years ago by Owen Arnold
This has been fantastically well implemented. The IOC is flawless, it is well tested, it works exactly as I would expect, and it produces the same (actually better) results than the old interface.
comment:40 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 11234