Ticket #10392 (closed: fixed)

Opened 6 years ago

Last modified 5 years ago

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.

  1. 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.
  2. The functionality should continue to provide good and useful feedback such as allowing the user to login to ICAT in situ.
  3. 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:1 Changed 6 years ago by Owen Arnold

  • Status changed from new to assigned

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:3 Changed 6 years ago by Harry Jeffery

  • Status changed from assigned to inprogress

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
  • Verify that the right-click menu also works for transferring runs
  • Verify all CustomInterfaces tests are passing
Last edited 6 years ago by Harry Jeffery (previous) (diff)

comment:32 Changed 6 years ago by Harry Jeffery

  • Status changed from verify to reopened
  • Resolution fixed deleted

comment:33 Changed 6 years ago by Harry Jeffery

  • Status changed from reopened to inprogress

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:39 Changed 6 years ago by Owen Arnold

  • Blocking 10540 added

comment:40 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 11234

Note: See TracTickets for help on using tickets.