From be08335f2c038dca24d46c983399ac3ea53f6f30 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 7 Jun 2012 13:54:37 +1000 Subject: [PATCH 1/7] simplify history use in SearchResultsPresenter --- .../client/presenter/SearchResultsPresenter.java | 16 ++++++++++------ .../client/view/SearchResultsView.ui.xml | 1 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java index dfea12cb42..9efc473a77 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java @@ -120,6 +120,10 @@ public interface Display extends WidgetDisplay HasChangeHandlers getSearchFieldSelector(); + static String SEARCH_FIELD_TARGET = "target"; + static String SEARCH_FIELD_SOURCE = "source"; + static String SEARCH_FIELD_BOTH = "both"; + String getSelectedSearchField(); void setSearching(boolean searching); @@ -280,7 +284,7 @@ public void onValueChange(ValueChangeEvent event) if (!event.getValue().equals(token.getProjectSearchReplacement())) { token.setProjectSearchReplacement(event.getValue()); - history.newItem(token.toTokenString()); + history.newItem(token); } } })); @@ -508,7 +512,7 @@ private void showDocInEditor(String doc, boolean runSearch) { token.setSearchText(""); } - history.newItem(token.toTokenString()); + history.newItem(token); } @Override @@ -1319,7 +1323,7 @@ private void refreshReplacementEventInfoList() private void updateSearch() { boolean changed = false; - HistoryToken token = HistoryToken.fromTokenString(history.getToken()); + HistoryToken token = history.getHistoryToken(); Boolean caseSensitive = display.getCaseSensitiveChk().getValue(); if (caseSensitive != token.getProjectSearchCaseSensitive()) @@ -1336,8 +1340,8 @@ private void updateSearch() } String selected = display.getSelectedSearchField(); - boolean searchSource = selected.equals("source") || selected.equals("both"); - boolean searchTarget = selected.equals("target") || selected.equals("both"); + boolean searchSource = selected.equals(Display.SEARCH_FIELD_SOURCE) || selected.equals(Display.SEARCH_FIELD_BOTH); + boolean searchTarget = selected.equals(Display.SEARCH_FIELD_TARGET) || selected.equals(Display.SEARCH_FIELD_BOTH); if (searchSource != token.isProjectSearchInSource()) { token.setProjectSearchInSource(searchSource); @@ -1351,7 +1355,7 @@ private void updateSearch() if (changed) { - history.newItem(token.toTokenString()); + history.newItem(token); } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml index 82713e347f..492aa49461 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml @@ -13,6 +13,7 @@ Case sensitive + search target search source search both From 6d77ce22bbeee9c5249b4ca7c68e145e817de2b7 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 7 Jun 2012 13:55:46 +1000 Subject: [PATCH 2/7] add search button test for SearchResultsPresenter --- .../presenter/SearchResultsPresenterTest.java | 121 ++++++++++++++---- 1 file changed, 96 insertions(+), 25 deletions(-) diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java index 37945e9f02..b10f5868d9 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java @@ -30,8 +30,11 @@ import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.reset; import static org.easymock.EasyMock.verify; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import java.util.ArrayList; +import java.util.List; import net.customware.gwt.presenter.client.EventBus; @@ -44,6 +47,7 @@ import org.zanata.webtrans.client.events.WorkspaceContextUpdateEvent; import org.zanata.webtrans.client.events.WorkspaceContextUpdateEventHandler; import org.zanata.webtrans.client.history.History; +import org.zanata.webtrans.client.history.HistoryToken; import org.zanata.webtrans.client.history.Window; import org.zanata.webtrans.client.history.Window.Location; import org.zanata.webtrans.client.keys.KeyShortcut; @@ -52,6 +56,7 @@ import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.shared.model.WorkspaceContext; +import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.event.dom.client.HasClickHandlers; import com.google.gwt.event.logical.shared.ValueChangeHandler; @@ -78,6 +83,8 @@ public class SearchResultsPresenterTest private static final String TEST_MESSAGE_REPLACE_SELECTED_KEY_SHORTCUT = "Replace selected text flows"; private static final String TEST_MESSAGE_TOGGLE_ROW_ACTION_BUTTONS = "Toggle row action buttons"; + private static final String TEST_SEARCH_PHRASE = "search phrase"; + //object under test SearchResultsPresenter searchResultsPresenter; @@ -95,6 +102,8 @@ public class SearchResultsPresenterTest Location mockWindowLocation; WorkspaceContext mockWorkspaceContext; + HasValue mockCaseSensitiveChk; + HasValue mockFilterTextBox; HasClickHandlers mockReplaceAllButton; HasValue mockReplacementTextBox; HasValue mockRequirePreviewChk; @@ -104,6 +113,8 @@ public class SearchResultsPresenterTest HasText mockSelectionInfoLabel; + List> allCaptures; + Capture> capturedHistoryValueChangeHandler; Capture capturedReplaceAllButtonClickHandler; Capture> capturedReplacementTextBoxValueChangeHandler; @@ -114,7 +125,8 @@ public class SearchResultsPresenterTest Capture capturedTransUnitUpdatedEventHandler; Capture capturedWorkspaceContextUpdatedEventHandler; - private Capture capturedKeyShortcuts; + Capture capturedKeyShortcuts; + Capture capturedHistoryToken; @@ -139,6 +151,8 @@ private void createAllMocks() mockWindowLocation = createAndAddMock(Window.Location.class); mockWorkspaceContext = createAndAddMock(WorkspaceContext.class); + mockCaseSensitiveChk = createAndAddMock(HasValue.class); + mockFilterTextBox = createAndAddMock(HasValue.class); mockReplaceAllButton = createAndAddMock(HasClickHandlers.class); mockReplacementTextBox = createAndAddMock(HasValue.class); mockRequirePreviewChk = createAndAddMock(HasValue.class); @@ -152,18 +166,31 @@ private void createAllMocks() private void createAllCaptures() { - capturedHistoryValueChangeHandler = new Capture>(); - capturedReplaceAllButtonClickHandler = new Capture(); - capturedReplacementTextBoxValueChangeHandler = new Capture>(); - capturedRequirePreviewChkValueChangeHandler = new Capture>(); - capturedSearchButtonClickHandler = new Capture(); - capturedSelectAllButtonClickHandler = new Capture(); - capturedSelectAllChkValueChangeHandler = new Capture>(); - - capturedTransUnitUpdatedEventHandler = new Capture(); - capturedWorkspaceContextUpdatedEventHandler = new Capture(); - - capturedKeyShortcuts = new Capture(); + allCaptures = new ArrayList>(); + + capturedHistoryValueChangeHandler = addCapture(new Capture>()); + capturedReplaceAllButtonClickHandler = addCapture(new Capture()); + capturedReplacementTextBoxValueChangeHandler = addCapture(new Capture>()); + capturedRequirePreviewChkValueChangeHandler = addCapture(new Capture>()); + capturedSearchButtonClickHandler = addCapture(new Capture()); + capturedSelectAllButtonClickHandler = addCapture(new Capture()); + capturedSelectAllChkValueChangeHandler = addCapture(new Capture>()); + + capturedTransUnitUpdatedEventHandler = addCapture(new Capture()); + capturedWorkspaceContextUpdatedEventHandler = addCapture(new Capture()); + + capturedKeyShortcuts = addCapture(new Capture()); + capturedHistoryToken = addCapture(new Capture()); + } + + /** + * Convenience method to add a capture to allCaptures + * @return the given capture for assignment + */ + private Capture addCapture(Capture capture) + { + allCaptures.add(capture); + return capture; } private T createAndAddMock(Class clazz) @@ -194,6 +221,53 @@ public void testExpectedActionsOnBind() verifyAllMocks(); } + public void searchButtonClickUpdatesHistory() + { + boolean caseSensitiveFalseValue = false; + + HistoryToken historyToken = searchPageHistoryToken(); + expect(mockHistory.getHistoryToken()).andReturn(historyToken).once(); + expect(mockCaseSensitiveChk.getValue()).andReturn(caseSensitiveFalseValue).once(); + expect(mockFilterTextBox.getValue()).andReturn(TEST_SEARCH_PHRASE).once(); + expect(mockDisplay.getSelectedSearchField()).andReturn(SearchResultsPresenter.Display.SEARCH_FIELD_TARGET).once(); + + mockHistory.newItem(capture(capturedHistoryToken)); + expectLastCall().once(); + replayAllMocks(); + + searchResultsPresenter.bind(); + // not sure if this is set to visible yet, need to look at that + + ClickEvent event = new ClickEvent() + { + }; + capturedSearchButtonClickHandler.getValue().onClick(event); + + verifyAllMocks(); + + HistoryToken newToken = capturedHistoryToken.getValue(); + assertThat("new history token should be updated with current search phrase in search text box", + newToken.getProjectSearchText(), is(TEST_SEARCH_PHRASE)); + assertThat("new history token project search case sensitivity should match checkbox value", + newToken.getProjectSearchCaseSensitive(), is(caseSensitiveFalseValue)); + assertThat("new history token should reflect search in target when selected search field is target", + newToken.isProjectSearchInTarget(), is(true)); + assertThat("new history token should reflect not to search in source when selected search field is target", + newToken.isProjectSearchInSource(), is(false)); + } + + /** + * @return + */ + private HistoryToken searchPageHistoryToken() + { + HistoryToken historyToken = new HistoryToken(); + historyToken.setView(MainView.Search); + return historyToken; + } + + + private void setupDefaultMockExpectations() { boolean workspaceIsReadOnly = false; @@ -230,6 +304,7 @@ private void expectUiMessages() private void expectDisplayComponentGetters() { + // getters used during bind expect(mockDisplay.getSelectionInfoLabel()).andReturn(mockSelectionInfoLabel).anyTimes(); expect(mockDisplay.getSearchButton()).andReturn(mockSearchButton).anyTimes(); expect(mockDisplay.getReplacementTextBox()).andReturn(mockReplacementTextBox).anyTimes(); @@ -237,6 +312,10 @@ private void expectDisplayComponentGetters() expect(mockDisplay.getRequirePreviewChk()).andReturn(mockRequirePreviewChk).anyTimes(); expect(mockDisplay.getReplaceAllButton()).andReturn(mockReplaceAllButton).anyTimes(); expect(mockDisplay.getSelectAllButton()).andReturn(mockSelectAllButton).anyTimes(); + + // getters used after bind + expect(mockDisplay.getCaseSensitiveChk()).andReturn(mockCaseSensitiveChk).anyTimes(); + expect(mockDisplay.getFilterTextBox()).andReturn(mockFilterTextBox).anyTimes(); } private void expectHandlerRegistrations() @@ -280,18 +359,10 @@ private void expectEventHandlerRegistration(Type exp private void resetAllCaptures() { - capturedHistoryValueChangeHandler.reset(); - capturedReplaceAllButtonClickHandler.reset(); - capturedReplacementTextBoxValueChangeHandler.reset(); - capturedRequirePreviewChkValueChangeHandler.reset(); - capturedSearchButtonClickHandler.reset(); - capturedSelectAllButtonClickHandler.reset(); - capturedSelectAllChkValueChangeHandler.reset(); - - capturedTransUnitUpdatedEventHandler.reset(); - capturedWorkspaceContextUpdatedEventHandler.reset(); - - capturedKeyShortcuts.reset(); + for (Capture capture : allCaptures) + { + capture.reset(); + } } private void resetAllMocks() From 3d78a3d294f50e3e7b72c385c8139f72bc898924 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 7 Jun 2012 14:52:34 +1000 Subject: [PATCH 3/7] add tests for search parameters in SearchResultsPresenter --- .../presenter/SearchResultsPresenterTest.java | 83 +++++++++++++++---- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java index b10f5868d9..a3e8517291 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java @@ -224,24 +224,10 @@ public void testExpectedActionsOnBind() public void searchButtonClickUpdatesHistory() { boolean caseSensitiveFalseValue = false; - - HistoryToken historyToken = searchPageHistoryToken(); - expect(mockHistory.getHistoryToken()).andReturn(historyToken).once(); - expect(mockCaseSensitiveChk.getValue()).andReturn(caseSensitiveFalseValue).once(); - expect(mockFilterTextBox.getValue()).andReturn(TEST_SEARCH_PHRASE).once(); - expect(mockDisplay.getSelectedSearchField()).andReturn(SearchResultsPresenter.Display.SEARCH_FIELD_TARGET).once(); - - mockHistory.newItem(capture(capturedHistoryToken)); - expectLastCall().once(); + expectRunSearch(searchPageHistoryToken(), caseSensitiveFalseValue, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_TARGET); replayAllMocks(); - searchResultsPresenter.bind(); - // not sure if this is set to visible yet, need to look at that - - ClickEvent event = new ClickEvent() - { - }; - capturedSearchButtonClickHandler.getValue().onClick(event); + simulateClick(capturedSearchButtonClickHandler); verifyAllMocks(); @@ -256,8 +242,52 @@ public void searchButtonClickUpdatesHistory() newToken.isProjectSearchInSource(), is(false)); } + public void searchWithCaseSensitive() + { + boolean caseSensitiveTrueValue = true; + expectRunSearch(searchPageHistoryToken(), caseSensitiveTrueValue, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_TARGET); + replayAllMocks(); + searchResultsPresenter.bind(); + simulateClick(capturedSearchButtonClickHandler); + verifyAllMocks(); + + HistoryToken newToken = capturedHistoryToken.getValue(); + assertThat("new history token project search case sensitivity should match checkbox value", + newToken.getProjectSearchCaseSensitive(), is(caseSensitiveTrueValue)); + } + + public void searchInSource() + { + expectRunSearch(searchPageHistoryToken(), false, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_SOURCE); + replayAllMocks(); + searchResultsPresenter.bind(); + simulateClick(capturedSearchButtonClickHandler); + verifyAllMocks(); + + HistoryToken newToken = capturedHistoryToken.getValue(); + assertThat("new history token should reflect search in source when selected search field is source", + newToken.isProjectSearchInSource(), is(true)); + assertThat("new history token should reflect not to search in target when selected search field is source", + newToken.isProjectSearchInTarget(), is(false)); + } + + public void searchInSourceAndTarget() + { + expectRunSearch(searchPageHistoryToken(), false, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_BOTH); + replayAllMocks(); + searchResultsPresenter.bind(); + simulateClick(capturedSearchButtonClickHandler); + verifyAllMocks(); + + HistoryToken newToken = capturedHistoryToken.getValue(); + assertThat("new history token should reflect search in source when selected search field is both", + newToken.isProjectSearchInSource(), is(true)); + assertThat("new history token should reflect search in target when selected search field is both", + newToken.isProjectSearchInTarget(), is(true)); + } + /** - * @return + * @return a history token with defaults except current page is search page */ private HistoryToken searchPageHistoryToken() { @@ -266,7 +296,13 @@ private HistoryToken searchPageHistoryToken() return historyToken; } - + private void simulateClick(Capture clickable) + { + ClickEvent event = new ClickEvent() + { + }; + clickable.getValue().onClick(event); + } private void setupDefaultMockExpectations() { @@ -357,6 +393,17 @@ private void expectEventHandlerRegistration(Type exp expect(mockEventBus.addHandler(eq(expectedType), and(capture(handlerCapture), isA(expectedClass)))).andReturn(createMock(HandlerRegistration.class)).once(); } + private void expectRunSearch(HistoryToken previousHistoryToken, boolean caseSensitive, String searchPhrase, String searchInFields) + { + expect(mockHistory.getHistoryToken()).andReturn(previousHistoryToken).once(); + expect(mockCaseSensitiveChk.getValue()).andReturn(caseSensitive).once(); + expect(mockFilterTextBox.getValue()).andReturn(searchPhrase).once(); + expect(mockDisplay.getSelectedSearchField()).andReturn(searchInFields).once(); + + mockHistory.newItem(capture(capturedHistoryToken)); + expectLastCall().once(); + } + private void resetAllCaptures() { for (Capture capture : allCaptures) From 0411f3ba2c840aff5b23f3f47fa1a9e36c730f6d Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 7 Jun 2012 16:50:25 +1000 Subject: [PATCH 4/7] move replacement box to top of search-replace page --- .../presenter/SearchResultsPresenter.java | 19 -------- .../client/view/SearchResultsView.java | 14 +----- .../client/view/SearchResultsView.ui.xml | 46 +++++++++---------- .../zanata/webtrans/public/Application.css | 43 +++++++---------- .../presenter/SearchResultsPresenterTest.java | 13 ------ 5 files changed, 41 insertions(+), 94 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java index 9efc473a77..22786d8fed 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java @@ -112,8 +112,6 @@ public interface Display extends WidgetDisplay void focusReplacementTextBox(); - HasText getSelectionInfoLabel(); - HasValue getCaseSensitiveChk(); HasValue getSelectAllChk(); @@ -138,8 +136,6 @@ public interface Display extends WidgetDisplay void setRequirePreview(boolean required); - HasClickHandlers getSelectAllButton(); - void clearAll(); /** @@ -334,19 +330,6 @@ public void onClick(ClickEvent event) } })); - - // TODO check "select entire document" checkbox if all rows individually - // selected (and clear for none selected) - registerHandler(display.getSelectAllButton().addClickHandler(new ClickHandler() - { - - @Override - public void onClick(ClickEvent event) - { - selectAllTextFlows(); - } - })); - history.addValueChangeHandler(new ValueChangeHandler() { @@ -667,7 +650,6 @@ public void onSelectionChange(SelectionChangeEvent event) } else { - display.getSelectionInfoLabel().setText(messages.numTextFlowsSelected(selectedFlows)); refreshReplaceAllButton(); } } @@ -1210,7 +1192,6 @@ private void clearAllExistingData() private void setUiForNothingSelected() { - display.getSelectionInfoLabel().setText(messages.noTextFlowsSelected()); display.setReplaceAllButtonEnabled(false); } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java index e98b10e2bc..10c6854c47 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java @@ -76,7 +76,7 @@ interface SearchResultsViewUiBinder extends UiBinder getCaseSensitiveChk() { @@ -222,12 +216,6 @@ public void setRequirePreview(boolean required) SearchResultsDocumentTable.setRequirePreview(required); } - @Override - public HasClickHandlers getSelectAllButton() - { - return selectAllLink; - } - @Override public HasText getSearchResponseLabel() { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml index 492aa49461..c8e6092bea 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.ui.xml @@ -6,7 +6,7 @@ xmlns:c="urn:import:com.google.gwt.user.cellview.client"> - + Search @@ -24,38 +24,38 @@ + + + + + + + + + + + Replace + + Require preview + + + + + Select all - + - - - Replace with: - - - - - - - - - select all - - - - Replace - - - Require preview - - + + + diff --git a/zanata-war/src/main/resources/org/zanata/webtrans/public/Application.css b/zanata-war/src/main/resources/org/zanata/webtrans/public/Application.css index 1e2e032e65..4aa90ecf5b 100644 --- a/zanata-war/src/main/resources/org/zanata/webtrans/public/Application.css +++ b/zanata-war/src/main/resources/org/zanata/webtrans/public/Application.css @@ -479,7 +479,8 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit color: #999999; background-color: #f3f2f2; border-bottom: 1px solid lightgrey; - padding-left: 20px; + padding: 0px 20px; + white-space: nowrap; } .gwt-ListBox @@ -513,6 +514,17 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit padding: 2px 8px; } +.projectWideReplacementBox +{ + padding: 4px; + font-size: 1.8em; + border: 4px solid lightgrey; + border-radius: 10px; + -moz-border-radius: 10px; + outline: none; + margin-right: 10px; +} + .projectWideSearchReportLabel { margin-left: 40px; @@ -525,7 +537,7 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit margin-left: 27px; } -.projectWideReplacementPanel +.projectWideSearchLogPanel { background-color: #667788; color: #ffffff; @@ -533,25 +545,6 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit padding: 0px 30px; } -.projectWideReplacementBoxLabel -{ - color: #ffffff; - font-size: 1.35em; - white-space: nowrap; -} - -.projectWideReplacementBox -{ - padding: 4px; - font-size: 1.8em; - border: 4px solid #667788; - border-radius: 10px; - -moz-border-radius: 10px; - outline: none; - margin-left: 15px; - margin-right: 35px; -} - .projectWideSearchSelectionInfo { font-size: 1.3em; @@ -562,11 +555,7 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit { color: #223344; padding: 3px 10px; - margin: 5px; font-size: 1.6em; - border-radius: 5px; - -moz-border-radius: 5px; - border-width: 1px; white-space: nowrap; } @@ -574,7 +563,9 @@ tr.ApprovedStateDecoration td.TableEditorCell-Target .TableEditorContent-Edit { color: #777777; background-color: #bbbbbb; - border: 1px solid #bbbbbb; + border: 2px solid #bbbbbb; + border-radius: 2px; + -moz-border-radius: 2px; } .projectWideReplaceFeedbackLabel diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java index a3e8517291..725093b996 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java @@ -63,7 +63,6 @@ import com.google.gwt.event.shared.EventHandler; import com.google.gwt.event.shared.HandlerRegistration; import com.google.gwt.event.shared.GwtEvent.Type; -import com.google.gwt.user.client.ui.HasText; import com.google.gwt.user.client.ui.HasValue; /** @@ -76,7 +75,6 @@ public class SearchResultsPresenterTest private static final int TOTAL_KEY_SHORTCUTS = 5; - private static final String TEST_MESSAGE_NO_TEXT_FLOWS_SELECTED = "No text flows selected"; private static final String TEST_MESSAGE_SELECT_ALL_TEXT_FLOWS_KEY_SHORTCUT = "Select all text flows"; private static final String TEST_MESSAGE_FOCUS_SEARCH_PHRASE_KEY_SHORTCUT = "Focus search phrase"; private static final String TEST_MESSAGE_FOCUS_REPLACEMENT_PHRASE_KEY_SHORTCUT = "Focus replacement phrase"; @@ -108,9 +106,7 @@ public class SearchResultsPresenterTest HasValue mockReplacementTextBox; HasValue mockRequirePreviewChk; HasClickHandlers mockSearchButton; - HasClickHandlers mockSelectAllButton; HasValue mockSelectAllChk; - HasText mockSelectionInfoLabel; List> allCaptures; @@ -157,9 +153,7 @@ private void createAllMocks() mockReplacementTextBox = createAndAddMock(HasValue.class); mockRequirePreviewChk = createAndAddMock(HasValue.class); mockSearchButton = createAndAddMock(HasClickHandlers.class); - mockSelectAllButton = createAndAddMock(HasClickHandlers.class); mockSelectAllChk = createAndAddMock(HasValue.class); - mockSelectionInfoLabel = createAndAddMock(HasText.class); allMocks = createdMocks.toArray(); } @@ -314,9 +308,6 @@ private void setupDefaultMockExpectations() expectDisplayComponentGetters(); - mockSelectionInfoLabel.setText(TEST_MESSAGE_NO_TEXT_FLOWS_SELECTED); - expectLastCall().once(); - mockDisplay.setReplaceAllButtonEnabled(false); expectLastCall().once(); @@ -330,7 +321,6 @@ private void setupDefaultMockExpectations() private void expectUiMessages() { - expect(mockMessages.noTextFlowsSelected()).andReturn(TEST_MESSAGE_NO_TEXT_FLOWS_SELECTED).anyTimes(); expect(mockMessages.selectAllTextFlowsKeyShortcut()).andReturn(TEST_MESSAGE_SELECT_ALL_TEXT_FLOWS_KEY_SHORTCUT).anyTimes(); expect(mockMessages.focusSearchPhraseKeyShortcut()).andReturn(TEST_MESSAGE_FOCUS_SEARCH_PHRASE_KEY_SHORTCUT).anyTimes(); expect(mockMessages.focusReplacementPhraseKeyShortcut()).andReturn(TEST_MESSAGE_FOCUS_REPLACEMENT_PHRASE_KEY_SHORTCUT).anyTimes(); @@ -341,13 +331,11 @@ private void expectUiMessages() private void expectDisplayComponentGetters() { // getters used during bind - expect(mockDisplay.getSelectionInfoLabel()).andReturn(mockSelectionInfoLabel).anyTimes(); expect(mockDisplay.getSearchButton()).andReturn(mockSearchButton).anyTimes(); expect(mockDisplay.getReplacementTextBox()).andReturn(mockReplacementTextBox).anyTimes(); expect(mockDisplay.getSelectAllChk()).andReturn(mockSelectAllChk).anyTimes(); expect(mockDisplay.getRequirePreviewChk()).andReturn(mockRequirePreviewChk).anyTimes(); expect(mockDisplay.getReplaceAllButton()).andReturn(mockReplaceAllButton).anyTimes(); - expect(mockDisplay.getSelectAllButton()).andReturn(mockSelectAllButton).anyTimes(); // getters used after bind expect(mockDisplay.getCaseSensitiveChk()).andReturn(mockCaseSensitiveChk).anyTimes(); @@ -361,7 +349,6 @@ private void expectHandlerRegistrations() expectValueChangeHandlerRegistration(mockSelectAllChk, capturedSelectAllChkValueChangeHandler); expectValueChangeHandlerRegistration(mockRequirePreviewChk, capturedRequirePreviewChkValueChangeHandler); expectClickHandlerRegistration(mockReplaceAllButton, capturedReplaceAllButtonClickHandler); - expectClickHandlerRegistration(mockSelectAllButton, capturedSelectAllButtonClickHandler); expect(mockHistory.addValueChangeHandler(capture(capturedHistoryValueChangeHandler))).andReturn(createMock(HandlerRegistration.class)).once(); From 1dd02bd3c4e2922b8e4d3ec33a24a742fa037e77 Mon Sep 17 00:00:00 2001 From: David Mason Date: Thu, 7 Jun 2012 17:01:47 +1000 Subject: [PATCH 5/7] add tooltip to 'require preview' checkbox on search-replace page --- .../org/zanata/webtrans/client/resources/WebTransMessages.java | 3 +++ .../org/zanata/webtrans/client/view/SearchResultsView.java | 1 + 2 files changed, 4 insertions(+) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java b/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java index e0451d9c41..83606519ba 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java @@ -331,4 +331,7 @@ public interface WebTransMessages extends Messages @DefaultMessage("Enter text to filter the document list. Use commas (,) to separate multiple searches") String docListFilterDescription(); + + @DefaultMessage("Disable 'Replace' button until previews have been generated for all selected text flows") + String requirePreviewDescription(); } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java index 10c6854c47..24456c1fd6 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java @@ -108,6 +108,7 @@ public SearchResultsView(Resources resources, final WebTransMessages webTransMes noResultsLabel.addStyleName("projectWideSearchNoResultsLabel"); searchResultsPanel.add(noResultsLabel); requirePreviewChk.setValue(true, false); + requirePreviewChk.setTitle(messages.requirePreviewDescription()); } @Override From f6a7382bf27bf9e35d58d36e93c321c95268a598 Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 8 Jun 2012 15:55:15 +1000 Subject: [PATCH 6/7] add tests for simple project-wide search execution --- .../presenter/SearchResultsPresenter.java | 37 +-- .../client/resources/WebTransMessages.java | 3 + .../client/view/SearchResultsView.java | 19 +- .../presenter/SearchResultsPresenterTest.java | 271 +++++++++++++++--- 4 files changed, 265 insertions(+), 65 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java index 22786d8fed..9709e6e637 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java @@ -71,7 +71,6 @@ import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwt.user.client.ui.HasText; import com.google.gwt.user.client.ui.HasValue; -import com.google.gwt.view.client.HasData; import com.google.gwt.view.client.ListDataProvider; import com.google.gwt.view.client.MultiSelectionModel; import com.google.gwt.view.client.SelectionChangeEvent; @@ -166,10 +165,10 @@ public interface Display extends WidgetDisplay * * @see SearchResultsDocumentTable#SearchResultsDocumentTable(SelectionModel, ValueChangeHandler, WebTransMessages) */ - HasData addDocument(String docName, + ListDataProvider addDocument(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, - SelectionModel selectionModel, + MultiSelectionModel selectionModel, ValueChangeHandler selectAllHandler); /** @@ -181,11 +180,11 @@ HasData addDocument(String docName, * @see #addDocument(String, ClickHandler, ClickHandler, SelectionModel, ValueChangeHandler) * @see SearchResultsDocumentTable#SearchResultsDocumentTable(Delegate, Delegate, Delegate, SelectionModel, ValueChangeHandler, WebTransMessages, org.zanata.webtrans.client.resources.Resources) */ - HasData addDocument( + ListDataProvider addDocument( String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, - SelectionModel selectionModel, + MultiSelectionModel selectionModel, ValueChangeHandler selectAllHandler, Delegate previewDelegate, Delegate replaceDelegate, @@ -276,7 +275,7 @@ public void onClick(ClickEvent event) @Override public void onValueChange(ValueChangeEvent event) { - HistoryToken token = HistoryToken.fromTokenString(history.getToken()); + HistoryToken token = history.getHistoryToken(); if (!event.getValue().equals(token.getProjectSearchReplacement())) { token.setProjectSearchReplacement(event.getValue()); @@ -535,7 +534,8 @@ public void onSuccess(GetProjectTransUnitListsResult result) int textFlowCount = displaySearchResults(result); if (result.getDocumentIds().size() == 0) { - display.getSearchResponseLabel().setText(""); + // TODO add case sensitivity and scope + display.getSearchResponseLabel().setText(messages.searchForPhraseReturnedNoResults(result.getSearchAction().getSearchString())); } else { @@ -1043,25 +1043,22 @@ private int displaySearchResults(GetProjectTransUnitListsResult result) */ private void displayDocumentResults(Long docId, final String docPathName, List transUnits) { + ListDataProvider dataProvider; final MultiSelectionModel selectionModel = new MultiSelectionModel(); - final ListDataProvider dataProvider = new ListDataProvider(); - documentDataProviders.put(docId, dataProvider); documentSelectionModels.put(docId, selectionModel); - - HasData table; ClickHandler showDocHandler = showDocClickHandler(docPathName, false); ClickHandler searchDocHandler = showDocClickHandler(docPathName, true); - ValueChangeHandler selectDocHandler = selectAllHandler(selectionModel, dataProvider); + ValueChangeHandler selectDocHandler = selectAllHandler(docId, selectionModel); if (showRowActionButtons) { - table = display.addDocument(docPathName, showDocHandler, searchDocHandler, selectionModel, selectDocHandler, + dataProvider = display.addDocument(docPathName, showDocHandler, searchDocHandler, selectionModel, selectDocHandler, ensurePreviewButtonDelegate(), ensureReplaceButtonDelegate(), ensureUndoButtonDelegate()); } else { - table = display.addDocument(docPathName, showDocHandler, searchDocHandler, selectionModel, selectDocHandler); + dataProvider = display.addDocument(docPathName, showDocHandler, searchDocHandler, selectionModel, selectDocHandler); } - dataProvider.addDataDisplay(table); + documentDataProviders.put(docId, dataProvider); selectionModel.addSelectionChangeHandler(selectionChangeHandler); @@ -1102,7 +1099,7 @@ public void onClick(ClickEvent event) * @param dataProvider * @return the new handler */ - private ValueChangeHandler selectAllHandler(final MultiSelectionModel selectionModel, final ListDataProvider dataProvider) + private ValueChangeHandler selectAllHandler(final Long docId, final MultiSelectionModel selectionModel) { return new ValueChangeHandler() { @@ -1111,9 +1108,13 @@ public void onValueChange(ValueChangeEvent event) { if (event.getValue()) { - for (TransUnitReplaceInfo info : dataProvider.getList()) + ListDataProvider dataProvider = documentDataProviders.get(docId); + if (dataProvider != null) { - selectionModel.setSelected(info, true); + for (TransUnitReplaceInfo info : dataProvider.getList()) + { + selectionModel.setSelected(info, true); + } } } else diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java b/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java index 83606519ba..4beadc5a02 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/resources/WebTransMessages.java @@ -147,6 +147,9 @@ public interface WebTransMessages extends Messages "other|one", "Showing results for search \"{0}\" ({1} text flows in 1 document)"}) String showingResultsForProjectWideSearch(String searchString, @PluralCount int textFlows, @PluralCount int documents); + @DefaultMessage("Search \"{0}\" returned no results") + String searchForPhraseReturnedNoResults(String searchString); + @DefaultMessage("There are no search results to display") String noSearchResults(); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java index 24456c1fd6..d090fd55f1 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java @@ -51,8 +51,8 @@ import com.google.gwt.user.client.ui.TextBox; import com.google.gwt.user.client.ui.VerticalPanel; import com.google.gwt.user.client.ui.Widget; -import com.google.gwt.view.client.HasData; -import com.google.gwt.view.client.SelectionModel; +import com.google.gwt.view.client.ListDataProvider; +import com.google.gwt.view.client.MultiSelectionModel; import com.google.inject.Inject; /** @@ -262,10 +262,10 @@ public void clearReplacementMessages() } @Override - public HasData addDocument(String docName, + public ListDataProvider addDocument(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, - SelectionModel selectionModel, + MultiSelectionModel selectionModel, ValueChangeHandler selectAllHandler) { SearchResultsDocumentTable table = new SearchResultsDocumentTable(selectionModel, selectAllHandler, messages); @@ -273,10 +273,10 @@ public HasData addDocument(String docName, } @Override - public HasData addDocument(String docName, + public ListDataProvider addDocument(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, - SelectionModel selectionModel, + MultiSelectionModel selectionModel, ValueChangeHandler selectAllHandler, Delegate previewDelegate, Delegate replaceDelegate, @@ -293,14 +293,17 @@ public HasData addDocument(String docName, * @param table * @return */ -private HasData addDocument(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, SearchResultsDocumentTable table) +private ListDataProvider addDocument(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler, SearchResultsDocumentTable table) { // ensure 'no results' message is no longer visible noResultsLabel.removeFromParent(); addDocumentLabel(docName, viewDocClickHandler, searchDocClickHandler); searchResultsPanel.add(table); table.addStyleName("projectWideSearchResultsDocumentBody"); - return table; + + ListDataProvider dataProvider = new ListDataProvider(); + dataProvider.addDataDisplay(table); + return dataProvider; } private void addDocumentLabel(String docName, ClickHandler viewDocClickHandler, ClickHandler searchDocClickHandler) diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java index 725093b996..ce057b2917 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java @@ -34,11 +34,15 @@ import static org.hamcrest.Matchers.is; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import net.customware.gwt.dispatch.shared.Action; import net.customware.gwt.presenter.client.EventBus; import org.easymock.Capture; +import org.easymock.IAnswer; import org.testng.annotations.BeforeClass; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -54,16 +58,24 @@ import org.zanata.webtrans.client.presenter.SearchResultsPresenter.Display; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; +import org.zanata.webtrans.shared.model.TransUnit; import org.zanata.webtrans.shared.model.WorkspaceContext; +import org.zanata.webtrans.shared.rpc.GetProjectTransUnitLists; +import org.zanata.webtrans.shared.rpc.GetProjectTransUnitListsResult; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.event.dom.client.HasClickHandlers; +import com.google.gwt.event.logical.shared.ValueChangeEvent; import com.google.gwt.event.logical.shared.ValueChangeHandler; import com.google.gwt.event.shared.EventHandler; import com.google.gwt.event.shared.HandlerRegistration; import com.google.gwt.event.shared.GwtEvent.Type; +import com.google.gwt.user.client.rpc.AsyncCallback; +import com.google.gwt.user.client.ui.HasText; import com.google.gwt.user.client.ui.HasValue; +import com.google.gwt.view.client.ListDataProvider; +import com.google.gwt.view.client.MultiSelectionModel; /** * @author David Mason, damason@redhat.com @@ -73,15 +85,32 @@ public class SearchResultsPresenterTest { + private static final String TEST_LOCALE_ID = "de"; + + private static final long TEST_DOC_ID_1 = 5L; + private static final String TEST_DOC_PATH_1 = "doc1"; + + private static final String TEST_TARGET_STRING_1 = "target 1"; + private static final String TEST_SOURCE_STRING_1 = "source"; + private static final int TEST_VERNUM_1 = 0; + private static final long TEST_TU_ID_1 = 7L; + private static final String TEST_RES_ID_1 = "resId1"; + + private static final int TOTAL_KEY_SHORTCUTS = 5; - private static final String TEST_MESSAGE_SELECT_ALL_TEXT_FLOWS_KEY_SHORTCUT = "Select all text flows"; private static final String TEST_MESSAGE_FOCUS_SEARCH_PHRASE_KEY_SHORTCUT = "Focus search phrase"; private static final String TEST_MESSAGE_FOCUS_REPLACEMENT_PHRASE_KEY_SHORTCUT = "Focus replacement phrase"; private static final String TEST_MESSAGE_REPLACE_SELECTED_KEY_SHORTCUT = "Replace selected text flows"; + private static final String TEST_MESSAGE_SELECT_ALL_TEXT_FLOWS_KEY_SHORTCUT = "Select all text flows"; + private static final String TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH = "Results for search XXX (X text flows in X documents)"; private static final String TEST_MESSAGE_TOGGLE_ROW_ACTION_BUTTONS = "Toggle row action buttons"; private static final String TEST_SEARCH_PHRASE = "search phrase"; + private static final String TEST_REPLACE_PHRASE = "replace phrase"; + + private static final String TEST_MESSAGE_NO_RESULTS_FOR_SEARCH = "No results for search XXX"; + //object under test @@ -106,6 +135,7 @@ public class SearchResultsPresenterTest HasValue mockReplacementTextBox; HasValue mockRequirePreviewChk; HasClickHandlers mockSearchButton; + HasText mockSearchResponseLabel; HasValue mockSelectAllChk; @@ -124,6 +154,18 @@ public class SearchResultsPresenterTest Capture capturedKeyShortcuts; Capture capturedHistoryToken; + Capture capturedDispatchedSearch; + Capture> capturedDispatchedSearchCallback; + + // arguments to Display.addDocument method + Capture capturedViewDocClickHandlers; + Capture capturedSearchDocClickHandlers; + Capture> capturedSelectionModels; + Capture > capturedSelectDocChangeHandlers; + + ListDataProvider mockDataProvider; + + @BeforeClass @@ -153,8 +195,11 @@ private void createAllMocks() mockReplacementTextBox = createAndAddMock(HasValue.class); mockRequirePreviewChk = createAndAddMock(HasValue.class); mockSearchButton = createAndAddMock(HasClickHandlers.class); + mockSearchResponseLabel = createAndAddMock(HasText.class); mockSelectAllChk = createAndAddMock(HasValue.class); + mockDataProvider = createAndAddMock(ListDataProvider.class); + allMocks = createdMocks.toArray(); } @@ -175,6 +220,14 @@ private void createAllCaptures() capturedKeyShortcuts = addCapture(new Capture()); capturedHistoryToken = addCapture(new Capture()); + + capturedDispatchedSearch = addCapture(new Capture()); + capturedDispatchedSearchCallback = addCapture(new Capture>()); + + capturedViewDocClickHandlers = addCapture(new Capture()); + capturedSearchDocClickHandlers = addCapture(new Capture()); + capturedSelectionModels = addCapture(new Capture>()); + capturedSelectDocChangeHandlers = addCapture(new Capture>()); } /** @@ -197,7 +250,7 @@ private T createAndAddMock(Class clazz) @BeforeMethod public void beforeMethod() { - resetAllMocks(); + reset(allMocks); resetAllCaptures(); setupDefaultMockExpectations(); @@ -210,20 +263,19 @@ public void beforeMethod() public void testExpectedActionsOnBind() { - replayAllMocks(); + replay(allMocks); searchResultsPresenter.bind(); - verifyAllMocks(); + verify(allMocks); } public void searchButtonClickUpdatesHistory() { boolean caseSensitiveFalseValue = false; expectRunSearch(searchPageHistoryToken(), caseSensitiveFalseValue, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_TARGET); - replayAllMocks(); + replay(allMocks); searchResultsPresenter.bind(); simulateClick(capturedSearchButtonClickHandler); - - verifyAllMocks(); + verify(allMocks); HistoryToken newToken = capturedHistoryToken.getValue(); assertThat("new history token should be updated with current search phrase in search text box", @@ -240,10 +292,10 @@ public void searchWithCaseSensitive() { boolean caseSensitiveTrueValue = true; expectRunSearch(searchPageHistoryToken(), caseSensitiveTrueValue, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_TARGET); - replayAllMocks(); + replay(allMocks); searchResultsPresenter.bind(); simulateClick(capturedSearchButtonClickHandler); - verifyAllMocks(); + verify(allMocks); HistoryToken newToken = capturedHistoryToken.getValue(); assertThat("new history token project search case sensitivity should match checkbox value", @@ -253,10 +305,10 @@ public void searchWithCaseSensitive() public void searchInSource() { expectRunSearch(searchPageHistoryToken(), false, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_SOURCE); - replayAllMocks(); + replay(allMocks); searchResultsPresenter.bind(); simulateClick(capturedSearchButtonClickHandler); - verifyAllMocks(); + verify(allMocks); HistoryToken newToken = capturedHistoryToken.getValue(); assertThat("new history token should reflect search in source when selected search field is source", @@ -268,10 +320,10 @@ public void searchInSource() public void searchInSourceAndTarget() { expectRunSearch(searchPageHistoryToken(), false, TEST_SEARCH_PHRASE, SearchResultsPresenter.Display.SEARCH_FIELD_BOTH); - replayAllMocks(); + replay(allMocks); searchResultsPresenter.bind(); simulateClick(capturedSearchButtonClickHandler); - verifyAllMocks(); + verify(allMocks); HistoryToken newToken = capturedHistoryToken.getValue(); assertThat("new history token should reflect search in source when selected search field is both", @@ -280,6 +332,157 @@ public void searchInSourceAndTarget() newToken.isProjectSearchInTarget(), is(true)); } + public void replacementValueChanged() + { + expect(mockHistory.getHistoryToken()).andReturn(searchPageHistoryToken()).once(); + expectNewHistoryItem(); + replay(allMocks); + searchResultsPresenter.bind(); + valueChangeEvent(capturedReplacementTextBoxValueChangeHandler, TEST_REPLACE_PHRASE); + verify(allMocks); + + HistoryToken newToken = capturedHistoryToken.getValue(); + assertThat("new history token should be updated with current replacement phrase", + newToken.getProjectSearchReplacement(), is(TEST_REPLACE_PHRASE)); + } + + public void selectAllChkChecked() + { + // set up some search results + // capture the selection models + // simulate check + // check that all are selected + } + + public void selectAllChkUnchecked() + { + // set up some search results + // capture the selection models + // simulate uncheck + // check that all are selected + } + + public void firesSearchFromHistoryNoResults() + { + expectPrepareToDispatchSearch(TEST_SEARCH_PHRASE, false, null); + expectDispatchSearch(buildNoSearchResultsResponse()); + + // display search results + mockDisplay.clearAll(); + mockDisplay.setReplaceAllButtonEnabled(false); + + expect(mockMessages.searchForPhraseReturnedNoResults(TEST_SEARCH_PHRASE)).andReturn(TEST_MESSAGE_NO_RESULTS_FOR_SEARCH).once(); + mockSearchResponseLabel.setText(TEST_MESSAGE_NO_RESULTS_FOR_SEARCH); + mockDisplay.setSearching(false); + + replay(allMocks); + searchResultsPresenter.bind(); + + HistoryToken token = searchPageHistoryToken(); + token.setProjectSearchText(TEST_SEARCH_PHRASE); + valueChangeEvent(capturedHistoryValueChangeHandler, token.toTokenString()); + + verify(allMocks); + } + + public void firesSearchFromHistoryOneResult() + { + expectPrepareToDispatchSearch(TEST_SEARCH_PHRASE, false, null); + + expectDispatchSearch(buildSingleTextFlowResponse()); + + // display search results + mockDisplay.clearAll(); + mockDisplay.setReplaceAllButtonEnabled(false); + mockDisplay.setSearching(false); + expect(mockMessages.showingResultsForProjectWideSearch(TEST_SEARCH_PHRASE, 1, 1)).andReturn(TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH).once(); + mockSearchResponseLabel.setText(TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH); + + // display single document + expect(mockDisplay.addDocument(eq(TEST_DOC_PATH_1), + capture(capturedViewDocClickHandlers), + capture(capturedSearchDocClickHandlers), + capture(capturedSelectionModels), + capture(capturedSelectDocChangeHandlers))).andReturn(mockDataProvider).once(); + List dataProviderList = new ArrayList(); + expect(mockDataProvider.getList()).andReturn(dataProviderList).once(); + + replay(allMocks); + searchResultsPresenter.bind(); + + HistoryToken token = searchPageHistoryToken(); + token.setProjectSearchText(TEST_SEARCH_PHRASE); + valueChangeEvent(capturedHistoryValueChangeHandler, token.toTokenString()); + + verify(allMocks); + + assertThat("text flows for document should be added to data provider", dataProviderList.size(), is(1)); + assertThat("new search results should not be selected initially", capturedSelectionModels.getValue().getSelectedSet().size(), is(0)); + } + + private IAnswer buildNoSearchResultsResponse() + { + return buildSuccessSearchResponse(new HashMap(), new HashMap>()); + } + + private IAnswer buildSingleTextFlowResponse() + { + final Map docPaths = new HashMap(); + docPaths.put(TEST_DOC_ID_1, TEST_DOC_PATH_1); + + final Map> documents = new HashMap>(); + List docs = new ArrayList(); + docs.add(TransUnit.Builder.newTransUnitBuilder() + .setId(TEST_TU_ID_1) + .setResId(TEST_RES_ID_1) + .setLocaleId(TEST_LOCALE_ID) + .setVerNum(TEST_VERNUM_1) + .addSource(TEST_SOURCE_STRING_1) + .addTargets(TEST_TARGET_STRING_1) + .build()); + documents.put(TEST_DOC_ID_1, docs); + + return buildSuccessSearchResponse(docPaths, documents); + } + + private IAnswer buildSuccessSearchResponse(final Map docPaths, final Map> documents) + { + IAnswer searchResponse = new IAnswer() + { + @Override + public GetProjectTransUnitListsResult answer() throws Throwable + { + GetProjectTransUnitListsResult result = new GetProjectTransUnitListsResult(capturedDispatchedSearch.getValue(), docPaths, documents); + capturedDispatchedSearchCallback.getValue().onSuccess(result); + return null; + } + }; + return searchResponse; + } + + private void expectPrepareToDispatchSearch(String searchPhrase, boolean caseSensitiveSearch, List queryStringDocuments) + { + // respond to search in history token + mockFilterTextBox.setValue(searchPhrase, false); + mockCaseSensitiveChk.setValue(caseSensitiveSearch, false); + mockDisplay.clearAll(); + mockDisplay.setReplaceAllButtonEnabled(false); + mockDisplay.setSearching(true); + + // this would probably be more correct to do when + // the search returns, rather than when it begins + mockDisplay.setHighlightString(searchPhrase); + + expect(mockWindowLocation.getQueryDocuments()).andReturn(queryStringDocuments).once(); + } + + @SuppressWarnings("unchecked") + private void expectDispatchSearch(IAnswer searchResponse) + { + mockDispatcher.execute(and(capture(capturedDispatchedSearch), isA(Action.class)), and(capture(capturedDispatchedSearchCallback), isA(AsyncCallback.class))); + expectLastCall().andAnswer(searchResponse); + } + /** * @return a history token with defaults except current page is search page */ @@ -298,24 +501,24 @@ private void simulateClick(Capture clickable) clickable.getValue().onClick(event); } + private void valueChangeEvent(Capture> handler, T newValue) + { + handler.getValue().onValueChange(new ValueChangeEvent(newValue) + { + }); + } + private void setupDefaultMockExpectations() { boolean workspaceIsReadOnly = false; expectUiMessages(); - - expect(mockWorkspaceContext.isReadOnly()).andReturn(workspaceIsReadOnly).anyTimes(); - expectDisplayComponentGetters(); - - mockDisplay.setReplaceAllButtonEnabled(false); - expectLastCall().once(); - - mockDisplay.setReplaceAllButtonVisible(!workspaceIsReadOnly); - expectLastCall().once(); - expectHandlerRegistrations(); + expect(mockWorkspaceContext.isReadOnly()).andReturn(workspaceIsReadOnly).anyTimes(); + mockDisplay.setReplaceAllButtonVisible(!workspaceIsReadOnly); + mockDisplay.setReplaceAllButtonEnabled(false); expect(mockKeyShortcutPresenter.registerKeyShortcut(capture(capturedKeyShortcuts))).andReturn(createMock(HandlerRegistration.class)).times(TOTAL_KEY_SHORTCUTS); } @@ -340,6 +543,7 @@ private void expectDisplayComponentGetters() // getters used after bind expect(mockDisplay.getCaseSensitiveChk()).andReturn(mockCaseSensitiveChk).anyTimes(); expect(mockDisplay.getFilterTextBox()).andReturn(mockFilterTextBox).anyTimes(); + expect(mockDisplay.getSearchResponseLabel()).andReturn(mockSearchResponseLabel).anyTimes(); } private void expectHandlerRegistrations() @@ -387,8 +591,12 @@ private void expectRunSearch(HistoryToken previousHistoryToken, boolean caseSens expect(mockFilterTextBox.getValue()).andReturn(searchPhrase).once(); expect(mockDisplay.getSelectedSearchField()).andReturn(searchInFields).once(); + expectNewHistoryItem(); + } + + private void expectNewHistoryItem() + { mockHistory.newItem(capture(capturedHistoryToken)); - expectLastCall().once(); } private void resetAllCaptures() @@ -398,19 +606,4 @@ private void resetAllCaptures() capture.reset(); } } - - private void resetAllMocks() - { - reset(allMocks); - } - - private void replayAllMocks() - { - replay(allMocks); - } - - private void verifyAllMocks() - { - verify(allMocks); - } } From f803d87a8fcd7b6d3bec53d380842f6fae25a199 Mon Sep 17 00:00:00 2001 From: David Mason Date: Fri, 8 Jun 2012 16:54:41 +1000 Subject: [PATCH 7/7] add test for select-all checkbox in SearchResultsPresenter --- .../presenter/SearchResultsPresenter.java | 9 +- .../client/view/SearchResultsView.java | 6 + .../presenter/SearchResultsPresenterTest.java | 127 ++++++++++++------ 3 files changed, 98 insertions(+), 44 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java index 9709e6e637..16f270e8fc 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/SearchResultsPresenter.java @@ -189,6 +189,13 @@ ListDataProvider addDocument( Delegate previewDelegate, Delegate replaceDelegate, Delegate undoDelegate); + + /** + * Required to avoid instantiating a component that calls client-only code in the presenter + * + * @return a new selection model for use with table + */ + MultiSelectionModel createMultiSelectionModel(); } private final WebTransMessages messages; @@ -1044,7 +1051,7 @@ private int displaySearchResults(GetProjectTransUnitListsResult result) private void displayDocumentResults(Long docId, final String docPathName, List transUnits) { ListDataProvider dataProvider; - final MultiSelectionModel selectionModel = new MultiSelectionModel(); + final MultiSelectionModel selectionModel = display.createMultiSelectionModel(); documentSelectionModels.put(docId, selectionModel); ClickHandler showDocHandler = showDocClickHandler(docPathName, false); ClickHandler searchDocHandler = showDocClickHandler(docPathName, true); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java index d090fd55f1..9c582bff89 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/SearchResultsView.java @@ -346,4 +346,10 @@ public void setSearching(boolean searching) searchingIndicator.hide(); } } + + @Override + public MultiSelectionModel createMultiSelectionModel() + { + return new MultiSelectionModel(); + } } diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java index ce057b2917..41ddf050be 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/SearchResultsPresenterTest.java @@ -35,6 +35,7 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; @@ -76,6 +77,7 @@ import com.google.gwt.user.client.ui.HasValue; import com.google.gwt.view.client.ListDataProvider; import com.google.gwt.view.client.MultiSelectionModel; +import com.google.gwt.view.client.SelectionChangeEvent; /** * @author David Mason, damason@redhat.com @@ -163,7 +165,16 @@ public class SearchResultsPresenterTest Capture> capturedSelectionModels; Capture > capturedSelectDocChangeHandlers; - ListDataProvider mockDataProvider; + // first test document + ListDataProvider mockDataProviderDoc1; + List dataProviderDoc1List; + MultiSelectionModel mockSelectionModelDoc1; + Capture capturedSelectedReplaceInfosDoc1; + Capture capturedSelectionChangeHandlerDoc1; + + + + @@ -198,7 +209,8 @@ private void createAllMocks() mockSearchResponseLabel = createAndAddMock(HasText.class); mockSelectAllChk = createAndAddMock(HasValue.class); - mockDataProvider = createAndAddMock(ListDataProvider.class); + mockDataProviderDoc1 = createAndAddMock(ListDataProvider.class); + mockSelectionModelDoc1 = createAndAddMock(MultiSelectionModel.class); allMocks = createdMocks.toArray(); } @@ -227,7 +239,10 @@ private void createAllCaptures() capturedViewDocClickHandlers = addCapture(new Capture()); capturedSearchDocClickHandlers = addCapture(new Capture()); capturedSelectionModels = addCapture(new Capture>()); + capturedSelectedReplaceInfosDoc1 = addCapture(new Capture()); capturedSelectDocChangeHandlers = addCapture(new Capture>()); + + capturedSelectionChangeHandlerDoc1 = addCapture(new Capture()); } /** @@ -252,6 +267,7 @@ public void beforeMethod() { reset(allMocks); resetAllCaptures(); + resetDataProviderLists(); setupDefaultMockExpectations(); @@ -261,6 +277,11 @@ public void beforeMethod() } + private void resetDataProviderLists() + { + dataProviderDoc1List = new ArrayList(); + } + public void testExpectedActionsOnBind() { replay(allMocks); @@ -346,22 +367,6 @@ public void replacementValueChanged() newToken.getProjectSearchReplacement(), is(TEST_REPLACE_PHRASE)); } - public void selectAllChkChecked() - { - // set up some search results - // capture the selection models - // simulate check - // check that all are selected - } - - public void selectAllChkUnchecked() - { - // set up some search results - // capture the selection models - // simulate uncheck - // check that all are selected - } - public void firesSearchFromHistoryNoResults() { expectPrepareToDispatchSearch(TEST_SEARCH_PHRASE, false, null); @@ -378,46 +383,75 @@ public void firesSearchFromHistoryNoResults() replay(allMocks); searchResultsPresenter.bind(); - HistoryToken token = searchPageHistoryToken(); - token.setProjectSearchText(TEST_SEARCH_PHRASE); - valueChangeEvent(capturedHistoryValueChangeHandler, token.toTokenString()); + simulateSearch(); verify(allMocks); } public void firesSearchFromHistoryOneResult() { - expectPrepareToDispatchSearch(TEST_SEARCH_PHRASE, false, null); + expectSearchAndDisplaySingleResult(); + replay(allMocks); + searchResultsPresenter.bind(); + simulateSearch(); + verify(allMocks); + assertThat("text flows for document should be added to data provider", dataProviderDoc1List.size(), is(1)); + } - expectDispatchSearch(buildSingleTextFlowResponse()); + // TODO use 4 results in 2 documents + public void selectAllChkCheckedSingleResult() + { + expectSearchAndDisplaySingleResult(); + mockSelectionModelDoc1.setSelected(capture(capturedSelectedReplaceInfosDoc1), eq(true)); - // display search results - mockDisplay.clearAll(); - mockDisplay.setReplaceAllButtonEnabled(false); - mockDisplay.setSearching(false); + replay(allMocks); + searchResultsPresenter.bind(); + simulateSearch(); + valueChangeEvent(capturedSelectAllChkValueChangeHandler, true); + + verify(allMocks); + } + + public void selectAllChkUnchecked() + { + // set up some search results + // capture the selection models + // simulate uncheck + // check that all are selected + } + + private void expectSearchAndDisplaySingleResult() + { + // set up some search results + expectPrepareToDispatchSearch(TEST_SEARCH_PHRASE, false, null); + expectDispatchSearch(buildSingleTextFlowResponse()); expect(mockMessages.showingResultsForProjectWideSearch(TEST_SEARCH_PHRASE, 1, 1)).andReturn(TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH).once(); - mockSearchResponseLabel.setText(TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH); + expectSearchReturned(TEST_MESSAGE_SHOWING_RESULTS_FOR_SEARCH); // display single document + expect(mockDisplay.createMultiSelectionModel()).andReturn(mockSelectionModelDoc1).once(); + expect(mockSelectionModelDoc1.addSelectionChangeHandler(capture(capturedSelectionChangeHandlerDoc1))).andReturn(mockHandlerRegistration()); + expect(mockDisplay.addDocument(eq(TEST_DOC_PATH_1), capture(capturedViewDocClickHandlers), capture(capturedSearchDocClickHandlers), capture(capturedSelectionModels), - capture(capturedSelectDocChangeHandlers))).andReturn(mockDataProvider).once(); - List dataProviderList = new ArrayList(); - expect(mockDataProvider.getList()).andReturn(dataProviderList).once(); + capture(capturedSelectDocChangeHandlers))).andReturn(mockDataProviderDoc1).once(); + } - replay(allMocks); - searchResultsPresenter.bind(); + private void expectSearchReturned(String searchResponseInfoMessage) + { + mockDisplay.clearAll(); + mockDisplay.setReplaceAllButtonEnabled(false); + mockDisplay.setSearching(false); + mockSearchResponseLabel.setText(searchResponseInfoMessage); + } + private void simulateSearch() + { HistoryToken token = searchPageHistoryToken(); token.setProjectSearchText(TEST_SEARCH_PHRASE); valueChangeEvent(capturedHistoryValueChangeHandler, token.toTokenString()); - - verify(allMocks); - - assertThat("text flows for document should be added to data provider", dataProviderList.size(), is(1)); - assertThat("new search results should not be selected initially", capturedSelectionModels.getValue().getSelectedSet().size(), is(0)); } private IAnswer buildNoSearchResultsResponse() @@ -516,10 +550,12 @@ private void setupDefaultMockExpectations() expectDisplayComponentGetters(); expectHandlerRegistrations(); + expect(mockDataProviderDoc1.getList()).andReturn(dataProviderDoc1List).anyTimes(); + expect(mockWorkspaceContext.isReadOnly()).andReturn(workspaceIsReadOnly).anyTimes(); mockDisplay.setReplaceAllButtonVisible(!workspaceIsReadOnly); mockDisplay.setReplaceAllButtonEnabled(false); - expect(mockKeyShortcutPresenter.registerKeyShortcut(capture(capturedKeyShortcuts))).andReturn(createMock(HandlerRegistration.class)).times(TOTAL_KEY_SHORTCUTS); + expect(mockKeyShortcutPresenter.registerKeyShortcut(capture(capturedKeyShortcuts))).andReturn(mockHandlerRegistration()).times(TOTAL_KEY_SHORTCUTS); } private void expectUiMessages() @@ -554,7 +590,7 @@ private void expectHandlerRegistrations() expectValueChangeHandlerRegistration(mockRequirePreviewChk, capturedRequirePreviewChkValueChangeHandler); expectClickHandlerRegistration(mockReplaceAllButton, capturedReplaceAllButtonClickHandler); - expect(mockHistory.addValueChangeHandler(capture(capturedHistoryValueChangeHandler))).andReturn(createMock(HandlerRegistration.class)).once(); + expect(mockHistory.addValueChangeHandler(capture(capturedHistoryValueChangeHandler))).andReturn(mockHandlerRegistration()).once(); expectEventHandlerRegistration(TransUnitUpdatedEvent.getType(), TransUnitUpdatedEventHandler.class, capturedTransUnitUpdatedEventHandler); expectEventHandlerRegistration(WorkspaceContextUpdateEvent.getType(), WorkspaceContextUpdateEventHandler.class, capturedWorkspaceContextUpdatedEventHandler); @@ -570,18 +606,23 @@ private void expectHandlerRegistrations() */ private void expectClickHandlerRegistration(HasClickHandlers mockObjectToClick, Capture captureForHandler) { - expect(mockObjectToClick.addClickHandler(and(capture(captureForHandler), isA(ClickHandler.class)))).andReturn(createMock(HandlerRegistration.class)).once(); + expect(mockObjectToClick.addClickHandler(and(capture(captureForHandler), isA(ClickHandler.class)))).andReturn(mockHandlerRegistration()).once(); } private void expectValueChangeHandlerRegistration(HasValue mockObjectWithValue, Capture> captureForHandler) { - expect(mockObjectWithValue.addValueChangeHandler(capture(captureForHandler))).andReturn(createMock(HandlerRegistration.class)).once(); + expect(mockObjectWithValue.addValueChangeHandler(capture(captureForHandler))).andReturn(mockHandlerRegistration()).once(); } //copied from AppPresenterTest private void expectEventHandlerRegistration(Type expectedType, Class expectedClass, Capture handlerCapture) { - expect(mockEventBus.addHandler(eq(expectedType), and(capture(handlerCapture), isA(expectedClass)))).andReturn(createMock(HandlerRegistration.class)).once(); + expect(mockEventBus.addHandler(eq(expectedType), and(capture(handlerCapture), isA(expectedClass)))).andReturn(mockHandlerRegistration()).once(); + } + + private HandlerRegistration mockHandlerRegistration() + { + return createMock(HandlerRegistration.class); } private void expectRunSearch(HistoryToken previousHistoryToken, boolean caseSensitive, String searchPhrase, String searchInFields)