Merge lp://qastaging/~suutari-olli/openlp/combined-bible-quick-search into lp://qastaging/openlp

Proposed by Azaziah
Status: Superseded
Proposed branch: lp://qastaging/~suutari-olli/openlp/combined-bible-quick-search
Merge into: lp://qastaging/openlp
Diff against target: 315 lines (+152/-63)
4 files modified
openlp/core/common/uistrings.py (+19/-0)
openlp/plugins/bibles/lib/manager.py (+31/-24)
openlp/plugins/bibles/lib/mediaitem.py (+101/-39)
resources/images/openlp-2.qrc (+1/-0)
To merge this branch: bzr merge lp://qastaging/~suutari-olli/openlp/combined-bible-quick-search
Reviewer Review Type Date Requested Status
Tim Bentley Needs Fixing
Tomas Groth Pending
Review via email: mp+290839@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2016-04-03.

This proposal has been superseded by a proposal from 2016-04-04.

Description of the change

This branch introduces following improvements to Quick Bible search:
- Combined Reference/Text search which first performs the Reference
  search and then moves to Text search if nothing is found.
- Possibility to use “.” when shortening Book names in Reference search.
  For an example Gen. 1 = Gen 1 = Genesis 1.
- New/Improved error messages (E.g. added actual example verses to Reference error)
  (Parts of the new messages are Bolded so <br> is required since \n does not work with bolding)

This branch also prevents users from performing Text searches which are:
- Shorter than 3 characters long (not including spaces)
- Searches consisting from only spaces

These currently possible bad search quarries result in LONG search times
and program instability/crashing. It’s still possible to search 3 characters
separated by spaces, but that scenario is relatively rarer.

No test(s) are yet included, if they must be, what should be tested?

lp:~suutari-olli/openlp/combined-bible-quick-search (revision 2619)
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-01-Pull/1320/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-02-Functional-Tests/1242/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-03-Interface-Tests/1181/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1016/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/607/
[←[1;32mSUCCESS←[1;m] https://ci.openlp.io/job/Branch-05a-Code_Analysis/674/
[←[1;31mFAILURE←[1;m] https://ci.openlp.io/job/Branch-05b-Test_Coverage/542/

To post a comment you must log in.
Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

Please don't include the changed resources.py in the merge request, it makes it impossible to see the code that has actually been changed.
Places the new image in "resources/images/" if it is not already there, but during the test/review process don't include the updated resources.py. We'll generate it locally for testing.

review: Needs Fixing
Revision history for this message
Tim Bentley (trb143) wrote :

Far to complex to translate and britle needs to be refactored to something simpler.

review: Needs Fixing
2623. By Azaziah

In this merge:

Refactored the code for combined search.
- Added: def on_quick_reference_search(self): and moved definition of reference search there.
- Added: def on_quick_text_search(self): and moved definition of text search there.
- Removed some un-needed code duplicates (Double finalizing, 3rd normalizing of mouse cursor)
- Searching scripture ref with shorter than 3 char search is now possible (G1 = Genesis 1)

Also removed “Search” from “Search Text or Reference…” since it does not fit the box properly.

2624. By Azaziah

Noticed one line was longer than 120, splitted it.

2625. By Azaziah

- Noticed I had left an old comment to a wrong place. (Moved it to def on_quick_reference_search)

2626. By Azaziah

In this merge:
Splitted scripture reference error into small pieces, built error from them.
This still needs to be moved away from uistrings.py

2627. By Azaziah

Noticed "No bibles available" was defined 3 times > Moved it to uistrings.
Added "Must have bibles" condition for other error messages in combined search.

To do:

Add a setting for controlling visibility of the no results error, it may be of annoyance.
Tests...

2628. By Azaziah

Added "Search Settings" > "Don't show error fofr no results in combined quick search" checkbox
for controlling whetever no results message is shown or not.

2629. By Azaziah

- Fixed layout for the new setting, cleanup for biblestab.py from process.

2630. By Azaziah

Merge to trunk on 9.4.16

2631. By Azaziah

Noticed the identification for scripture reference error got messed at some point, fixed it.
Also renamed "Interval" to "gap" to make constructing the message less painful.

2632. By Azaziah

In this commit:
- Cleanup of the scripture reference error. (Now using full names, removed one un-needed line)
- Added more comments
- Removed def get_verses_combined, now triggering the error message on mediaitem based on search mode instead.

2633. By Azaziah

Commit for merging trunk.

This also has a broken test.

2634. By Azaziah

Merged trunk on 28.4.16, removed broken test.

2635. By Azaziah

Added a setting for automatically resetting the Quick search type to "Combined" on startup.
This setting is by default enabled. (I Feel like some users may get confused if their search
has changed to Text or Reference)

2636. By Azaziah

Reworked UI messages on new settings, pep8 fixes.

2637. By Azaziah

Code cleanup, (Removed def banana)

2638. By Azaziah

Added a test for checking that all the general required stuff is called on quick search.

2639. By Azaziah

- Removed un-needed import that was added earlier
- Pep8 fixes

2640. By Azaziah

New line is now after ( in biblestab, hopefully this fixes pep8.

2641. By Azaziah

- Added "Search while typing" feature for Bible Quick search.
- Added a Setting for controlling this ^^ behaviour
- Refactored some code to avoid duplicate error messages

2642. By Azaziah

- Fixed the setting. (Was directed to another setting on one connection)
- Pep8

2643. By Azaziah

Improved one comment.

2644. By Azaziah

Removed # Cherry comment which I made earlier to locate some code.

2645. By Azaziah

Improved the UI message for the new setting.

2646. By Azaziah

- Renamed one new function to better describe the action
- Removed 3rd empty line after imports

2647. By Azaziah

Improved a bunch of comments.

2648. By Azaziah

Changed the requested python2 stuff into python 3 stuff.

2649. By Azaziah

Added . to the end of web bibles sentence.

is installed as Web Bible.

2650. By Azaziah

- Improved the timer method, resulting in much smoother searches.

2651. By Azaziah

- Merged trunk on 11.5.16

2652. By Azaziah

- Fixed the dot chechk method.
  (No longer removes dot after numbers, eg. 1.)

2653. By Azaziah

- Noticed this had a conflict, fixed it, improved comments

2654. By Azaziah

- Fixed Settings().Value to .value

2655. By Azaziah

- Fixed the test

2656. By Azaziah

Removed 2nd blank line from end of the uistrings.

2657. By Azaziah

- Merged trunk on 21.5.16

2658. By Azaziah

- Removed the unrequired %s complications from "No Bibles installed"

2659. By Azaziah

- Removed unrequired % s complications from short search messsage.

2660. By Azaziah

- Created "def verse_search_while_typing"
  / This does not trigger web bibles error message and is
    used during search while typing.
- Removed the new/old setting created for the old solution

2661. By Azaziah

- Merged trunk on 5.6.16
- Removed some additional error messages from "Search while typing"
  (Count verses not found in both bibles, no Bibles installed)

2662. By Azaziah

- Removed one old comment related to old setting and old solution

2663. By Azaziah

- Added "Search too short for search while typing" message to search results
  !! Need to rename this from: search_results_banana to something smarter.
- Fixed "," breaking OLP if it's the last char in text search.
- "," no longer separates keywords in Text search.
- Removed mention about separating keywords with "," from the empty search message.
- Started working on not duplicating search results on "Lock search results" > THIS DOES NOT WORK YET.

2664. By Azaziah

- Improved the "Too short search" item functionality.
- Improved some comments.

2665. By Azaziah

- Improved the "Search too short" message for Quick search.
 (Reference search just uses "No search results for now")
- Reverted the attempt to stop duplicated search results if search results are locked,
  (This is something that I havn't figured out how to do)

2666. By Azaziah

- Some code cleanup

2667. By Azaziah

- Merged trunk on 14.6.16

To do?
- Add clear search results button for locked Bible search results
- Prevent duplicated search results on "Lock search results"

2668. By Azaziah

- Added "Clear search results" button to Quick & Advanced search.
> Added a new icon for this: http://www.1001freedownloads.com/free-clipart/tango-edit-clear
 ^^CC0 1.0 Universal (CC0 1.0)

2669. By Azaziah

- Code cleanup (Removed 2 un-required empty rows)

2670. By Azaziah

- Added the Images by using "bzr add"

E:\bzr\openlp\combined-reference-and-text-search>bzr add E:\bzr\openlp\combined-
reference-and-text-search\resources\images
adding resources/images/bibles_search_clear.png
adding resources/images/bibles_search_combined.png

2671. By Azaziah

- Clear the results button now also clears the text input field and gives it focus.

2672. By Azaziah

Changed icons to oxygen icons.

2673. By Azaziah

- merged trunk on 24.7.16

2674. By Azaziah

- Merged trunk on 28.7.16

2675. By Azaziah

- Removed the clean icon manual sizing.

2676. By Azaziah

- Changed the Clear button to use QToolButton instead of QPushButton.
  (Fixes the issue where the clear button is larger than the lock button).

2677. By Azaziah

- Lock button now gives focus to Search field.

2678. By Azaziah

- Replaced the new icons with 48x48 versions.

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.