Customize selection for candidate needles + full diff view

[Martchus]

• Still WIP. Missing:
  • Select 'None' again
  • Actually implement 'Full diff'
  • Show whether needle is excluded
  • Fix tests
• Built on top of Migrate to Bootstrap 4 #1619 (comment).
• Can be tested on e212, eg. http://e212.suse.de/tests/11171#step/disable_grub_timeout/6.
• Main point is that the combo box content is now fully customizable.

[codecov]

Codecov Report

Merging #1622 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1622      +/-   ##
==========================================
+ Coverage   88.59%   88.62%   +0.02%     
==========================================
  Files         124      124              
  Lines        9270     9276       +6     
==========================================
+ Hits         8213     8221       +8     
+ Misses       1057     1055       -2

Impacted Files	Coverage Δ
lib/OpenQA/WebAPI/Controller/Step.pm	82.45% <100%> (+0.31%)	⬆️
lib/OpenQA/Worker/Engines/isotovideo.pm	95.12% <0%> (+1.21%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 585746f...b89dfa8. Read the comment docs.

[Martchus]

Full diff view looks now like this:

[coolo]

please rebase - hopefully the tests pass then too :)

[Martchus]

Rebased - if the tests pass that means we don't really tests that part of the UI yet. Maybe I should add some tests at least for the menu. And besides, I should also test this on staging first and you maybe want some adjustments.

[Martchus]

The automatic tests pass now. It can also be tested manually on e212, eg. under http://e212.suse.de/tests/11979#step/kontact/20 there's an example with multiple tags. The following should work now:

• So now there are 3 different views:
  • just the screenshot (previously the 'None' entry)
  • regular diff on matches (the only diff we had previously)
  • full diff: needle with original areas on left side, screenshot with matches on right side
• Usability is pretty much as it was before (menu closes again on selection, keyboard shortcuts).
• @okurz All text is selectable so you can copy tag and needle names.

---

The following things are still TODO but could also be improved in a further PR (so this one doesn't become too big):

• Allow to distinguish excluded needles.
• Add more extensive UI tests. Currently we only test whether candidates are displayed as expected but not really whether the diff actually works. Note that adding such tests would likely result in just adding checks for lots of internal variables used by the JavaScript. Or maybe we can extend the openQA-in-openQA test?
• Adjust details based on your feedback (like changing the button icons, change the wording used on tool tips, ...).
• Display even more information. The drop down is fully customizable now.
• Check whether documentation should be updated. Maybe we can also add tooltips or help popovers directly on the page.

[coolo]

Use this PR to update the tour - so we get feedback on the usefulness of this feature :)

[coolo]

and having openQA in openQA tests moving the screenshot slider sounds like a place where no man has gone before ;)

[Martchus]

Good idea. If we want to keep the tour feature, we should refactor it. Sticking with the current "coding style" would be very messy if we add more tours. However, for now I would keep it. Let's see what feedback we get first.

and having openQA in openQA tests moving the screenshot slider sounds like a place where no man has gone before ;)

Match the output in the initial state is likely enough. But if I wanted to move the slider, I'd likely try to inject some JavaScript code for that instead of moving the slider by hand :-)

[Martchus]

Now, that I've looked into it, I doubt that making a tour for this is easy and worth the effort. Things which make it difficult:

• The tour is currently only defined in JavaScript. However, it would be required to access the database from it. Ok, we could use AJAX and our existing rest API to find out a suitable and recent job (where results hopefully haven't been removed yet).
• When we find a suitable job, we would need to find suitable result screenshots. Since the interesting parts (available candidate needles) are lazy-loaded, the required information is not immediately available to the JavaScript. Hence more AJAX required.
• Or we implement the concept of 'tour fixtures'. So we can skip the mentioned effort and just run the tour on well known dummy data. That seems like the less error prone approach to me. But also effort to implement.
• The tour is implemented as a bootstrap popover. This very likely collides with showing a dropdown menu at the same time. At least I expect some extra effort for this to work.

To summarize: I don't think this feature is a good place to start for extending the tour feature. Especially when we're still investigating it and want to collect feedback. It would be much effort and likely we would provide a tour which doesn't work very well due to the complexity.

Note that these problems don't apply to all features we possibly want to show in a tour. The maintenance effort for the tour is also low (we've just migrated to Bootstrap 4). So I would keep the tour and try it out later when we introduce a new feature where it makes more sense.

If we later decide to keep the tour, I would opt for the 'tour fixtures' approach.

[coolo]

If the tour can't be used for such a thing - then let's drop it. Not in this PR though

[Martchus]

If the tour can't be used for such a thing

I'm not saying that. It would take extra effort like introducing 'tour fixtures'. And that does not make sense for a first test.

And the mentioned points really don't apply to any feature we might want to show. For instance, if we don't have to care which job to pick, we could just use /tests/latest. And introducing similar routes like /group_overview/any would be quite simple.

---

But I just want to clarify this. It is up to you to decide whether the tour is worth the effort.