From 240234495d95dec24003e6f277d4562ca97387fc Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 30 Aug 2013 14:09:26 +1000 Subject: [PATCH 01/39] rhbz1002774 - changed source plural will not generate invalid DTO --- .../main/java/org/zanata/model/HTextFlow.java | 11 +++++++ .../zanata/rest/service/ResourceUtils.java | 23 ++++++++++--- .../rpc/GetTranslationHistoryHandler.java | 8 ++++- .../rpc/GetTranslationHistoryHandlerTest.java | 32 +++++++++++++++++++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java index a59135b6d1..e7b1332ac4 100644 --- a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java +++ b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java @@ -73,6 +73,7 @@ import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; /** * Represents a flow of source text that should be processed as a stand-alone @@ -506,6 +507,16 @@ private void preUpdate() { this.getHistory().put(this.oldRevision, this.initialState); } + if (!isPlural()) + { + // if plural form has changed, we need to clear out obsolete contents + List newContents = Lists.newArrayList(content0); + for (int i = 1; i < MAX_PLURALS; i++) + { + newContents.add(null); + } + setContents(newContents); + } } } diff --git a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java index 16f1911474..d718688700 100644 --- a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java +++ b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java @@ -50,7 +50,6 @@ import org.zanata.model.HSimpleComment; import org.zanata.model.HTextFlow; import org.zanata.model.HTextFlowTarget; -import org.zanata.model.HasSimpleComment; import org.zanata.model.po.HPoHeader; import org.zanata.model.po.HPoTargetHeader; import org.zanata.model.po.HPotEntryData; @@ -1149,7 +1148,14 @@ protected void pullPoTargetComment(HPoTargetHeader fromHeader, PoTargetHeader to public void transferToTextFlow(HTextFlow from, TextFlow to) { - to.setContents(from.getContents()); + if (from.isPlural()) + { + to.setContents(from.getContents()); + } + else + { + to.setContents(from.getContents().get(0)); + } to.setRevision(from.getRevision()); to.setPlural(from.isPlural()); @@ -1311,9 +1317,16 @@ public String decodeDocId(String id) * @param apiVersion * @todo merge with {@link #transferToTextFlowTargetExtensions} */ - public void transferToTextFlowTarget(HTextFlowTarget from, TextFlowTarget to, Optional apiVersion) + public void transferToTextFlowTarget(HTextFlowTarget from, TextFlowTarget to, boolean isPlural, Optional apiVersion) { - to.setContents(from.getContents()); + if (isPlural) + { + to.setContents(from.getContents()); + } + else if (!from.getContents().isEmpty()) + { + to.setContents(from.getContents().get(0)); + } // TODO rhbz953734 - at the moment we will map review state into old state for compatibility to.setState(mapContentState(apiVersion, from.getState())); to.setRevision(from.getVersionNum()); @@ -1386,7 +1399,7 @@ public boolean transferToTranslationsResource(TranslationsResource transRes, HDo found = true; TextFlowTarget target = new TextFlowTarget(); target.setResId(hTarget.getTextFlow().getResId()); - this.transferToTextFlowTarget(hTarget, target, apiVersion); + this.transferToTextFlowTarget(hTarget, target, hTarget.getTextFlow().isPlural(), apiVersion); this.transferToTextFlowTargetExtensions(hTarget, target.getExtensions(true), enabledExtensions); transRes.getTextFlowTargets().add(target); } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTranslationHistoryHandler.java b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTranslationHistoryHandler.java index 2e18f474d1..62e9c83334 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTranslationHistoryHandler.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/server/rpc/GetTranslationHistoryHandler.java @@ -20,6 +20,7 @@ import org.zanata.model.HTextFlowTarget; import org.zanata.model.HTextFlowTargetHistory; import org.zanata.model.HTextFlowTargetReviewComment; +import org.zanata.rest.service.ResourceUtils; import org.zanata.security.ZanataIdentity; import org.zanata.service.LocaleService; import org.zanata.webtrans.server.ActionHandlerFor; @@ -55,6 +56,9 @@ public class GetTranslationHistoryHandler extends AbstractActionHandleremptyIterable()); + assertThat(result.getLatest().getVersionNum(), Matchers.equalTo(currentTranslation.getVersionNum().toString())); + assertThat(result.getLatest().getContents(), Matchers.contains(currentTranslation.getContents().get(0))); + assertThat(result.getLatest().getModifiedBy(), Matchers.equalTo("")); + } + private static HTextFlow createHTextFlow() { HTextFlow hTextFlow = new HTextFlow(); HashMap targetMap = Maps.newHashMap(); hTextFlow.setTargets(targetMap); + hTextFlow.setPlural(true); return hTextFlow; } From b4fcda0e18be8c05bf0d506e9b00460f9e96d76c Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 30 Aug 2013 14:48:58 +1000 Subject: [PATCH 02/39] update zanata-adapter-po version to match latest --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 21970a7656..19478e1bfb 100644 --- a/pom.xml +++ b/pom.xml @@ -36,7 +36,7 @@ 3.0.1 3.0.1 - 3.0.1 + 3.0.2-SNAPSHOT 4.3.2.Final From 35b8765d5b51aba5253c8428933228b0feeb8f60 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 2 Sep 2013 09:26:29 +1000 Subject: [PATCH 03/39] rhbz1002774 - minor refactor and fix HTextFlowHistoryJPATest --- .../main/java/org/zanata/rest/service/ResourceUtils.java | 6 +++--- .../test/java/org/zanata/model/HTextFlowHistoryJPATest.java | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java index d718688700..59867901e1 100644 --- a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java +++ b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java @@ -1317,9 +1317,9 @@ public String decodeDocId(String id) * @param apiVersion * @todo merge with {@link #transferToTextFlowTargetExtensions} */ - public void transferToTextFlowTarget(HTextFlowTarget from, TextFlowTarget to, boolean isPlural, Optional apiVersion) + public void transferToTextFlowTarget(HTextFlowTarget from, TextFlowTarget to, Optional apiVersion) { - if (isPlural) + if (from.getTextFlow().isPlural()) { to.setContents(from.getContents()); } @@ -1399,7 +1399,7 @@ public boolean transferToTranslationsResource(TranslationsResource transRes, HDo found = true; TextFlowTarget target = new TextFlowTarget(); target.setResId(hTarget.getTextFlow().getResId()); - this.transferToTextFlowTarget(hTarget, target, hTarget.getTextFlow().isPlural(), apiVersion); + this.transferToTextFlowTarget(hTarget, target, apiVersion); this.transferToTextFlowTargetExtensions(hTarget, target.getExtensions(true), enabledExtensions); transRes.getTextFlowTargets().add(target); } diff --git a/zanata-war/src/test/java/org/zanata/model/HTextFlowHistoryJPATest.java b/zanata-war/src/test/java/org/zanata/model/HTextFlowHistoryJPATest.java index 57b48b34cf..4f21bb3454 100644 --- a/zanata-war/src/test/java/org/zanata/model/HTextFlowHistoryJPATest.java +++ b/zanata-war/src/test/java/org/zanata/model/HTextFlowHistoryJPATest.java @@ -84,6 +84,7 @@ public void ensureHistoryIsRecordedPlural() session.flush(); HTextFlow tf = new HTextFlow(d, "mytf", "hello world"); + tf.setPlural(false); d.getTextFlows().add(tf); session.flush(); @@ -93,6 +94,7 @@ public void ensureHistoryIsRecordedPlural() assertThat("Incorrect History size on persist", historyElems.size(), is(0)); d.incrementRevision(); + tf.setPlural(true); tf.setContents("hello world 1", "hello world 2"); tf.setRevision(d.getRevision()); session.flush(); From ce5e911cb95eb4e712562879a21d3848b5e86fe8 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 2 Sep 2013 11:58:10 +1000 Subject: [PATCH 04/39] minor refactor --- .../src/main/java/org/zanata/rest/service/ResourceUtils.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java index 59867901e1..746eb33d93 100644 --- a/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java +++ b/zanata-war/src/main/java/org/zanata/rest/service/ResourceUtils.java @@ -1327,6 +1327,10 @@ else if (!from.getContents().isEmpty()) { to.setContents(from.getContents().get(0)); } + else + { + to.setContents(Collections.emptyList()); + } // TODO rhbz953734 - at the moment we will map review state into old state for compatibility to.setState(mapContentState(apiVersion, from.getState())); to.setRevision(from.getVersionNum()); From 2e832be3ee62f44e2c97c45f33443ce888163d68 Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 11:53:39 +1000 Subject: [PATCH 05/39] Refactor Copy Trans to be more testable. Correct a previously inserted bug where translations from obsolete documents where not being reused. Refactor Copy trans to be more functional and thus testable. Add some tests to test these discrete pieces. --- .../org/zanata/dao/TextFlowTargetDAO.java | 4 +- .../service/impl/CopyTransServiceImpl.java | 173 +++++++---- .../impl/CopyTransServiceImplTest.java | 281 ++++++++++++++---- 3 files changed, 345 insertions(+), 113 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/dao/TextFlowTargetDAO.java b/zanata-war/src/main/java/org/zanata/dao/TextFlowTargetDAO.java index 38f3697cf7..43808bc1c5 100644 --- a/zanata-war/src/main/java/org/zanata/dao/TextFlowTargetDAO.java +++ b/zanata-war/src/main/java/org/zanata/dao/TextFlowTargetDAO.java @@ -198,8 +198,8 @@ public ScrollableResults findMatchingTranslations(HDocument document, HLocale lo "or :finalState != (select t.state from HTextFlowTarget t where t.textFlow = textFlow and t.locale = :locale) ) " + // Do not reuse its own translations "and match.textFlow != textFlow " + - // Do not reuse matches from obsolete entities (document, iteration, project) - "and match.textFlow.document.obsolete = false " + + // Do not reuse matches from obsolete entities (iteration, project) + // Obsolete document translations ARE reused "and match.textFlow.document.projectIteration.status != :obsoleteEntityStatus " + "and match.textFlow.document.projectIteration.project.status != :obsoleteEntityStatus " ); diff --git a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java index 0dafc7bf89..a996f8eac9 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java @@ -26,7 +26,6 @@ import java.util.List; -import javax.annotation.Nullable; import javax.persistence.EntityManager; import org.hibernate.HibernateException; @@ -55,6 +54,10 @@ import org.zanata.rest.service.TranslatedDocResourceService; import org.zanata.service.CopyTransService; import org.zanata.service.LocaleService; +import com.google.common.collect.ImmutableList; + +import lombok.AllArgsConstructor; +import lombok.Getter; //TODO unit test suite for this class @@ -291,69 +294,104 @@ private int copyTransPass( HDocument document, HLocale locale, boolean checkCont } /** - * Determines the content state a copied translation should have. + * Determines the content state for a translation given a list of rules and their evaluation result, and + * the initial state that it was copied as. * - * @param contextMatches Whether the copied TextFlow's context matches the target TextFlow's context or not. - * @param projectMatches Whether the copied TextFlow's project matches the target TextFlow's context or not. - * @param docIdMatches Whether the copied TextFlow's docId matches the target TextFlow's context or not. - * @param options The copy trans options being used. - * @param requireTranslationReview Whether the project being copied to requires review or not. - * @param matchingTargetState The state of the match found by copy trans. - * @return The content state that the copied translation should have. May return null if the translation should not - * be copied at all. + * @param pairs List of evaluated rules and their result. + * @param initialState The initial content state of the translation that the content was copied from. + * @return The content state that the copied translation should have. 'New' indicates that the translation + * should not copied. */ - private @Nullable - ContentState determineContentState(boolean contextMatches, boolean projectMatches, boolean docIdMatches, - HCopyTransOptions options, boolean requireTranslationReview, ContentState matchingTargetState) + private static + ContentState determineContentStateFromMatchRules(List pairs, ContentState initialState) { - // Everything matches, and requires approval - if (requireTranslationReview && matchingTargetState.isApproved() && projectMatches && contextMatches && docIdMatches) + if( pairs.isEmpty() ) + { + return initialState; + } + + ResultActionPair p = pairs.get(0); + if( shouldReject(p.getMatchResult(), p.getRuleAction()) ) + { + return New; + } + else if( shouldDowngradeToFuzzy(p.getMatchResult(), p.getRuleAction()) ) { - return Approved; + return determineContentStateFromMatchRules(pairs.subList(1, pairs.size()), NeedReview); } - // Everything matches, and does not require approval - else if( matchingTargetState.isApproved() && projectMatches && contextMatches && docIdMatches ) + else { - return Translated; + return determineContentStateFromMatchRules(pairs.subList(1, pairs.size()), initialState); } - // Everything else - ContentState state = Translated; - state = getExpectedContentState(contextMatches, options.getContextMismatchAction(), state); - state = getExpectedContentState(projectMatches, options.getProjectMismatchAction(), state); - state = getExpectedContentState(docIdMatches, options.getDocIdMismatchAction(), state); - return state; } /** - * Gets the content state that is expected for a translation when evaluated against a single copy trans - * condition. - * Copy Trans conditions are of the form: Is it the same project? Is it the same documentId? ... etc. + * Determines the content state for a translation given a list of rules and their evaluation result. * - * @param match Whether the condition holds or not (is there a match?) - * @param action The action to take when the condition matches. - * @param currentState The current state of the translation. - * @return The content state that is expected the copied translation to have. + * @param pairs List of evaluated rules and their result. + * @param requireTranslationReview Whether the project to copy the translation to requires translations to be reviewed. + * @param matchingTargetState The initial state of the matching translation (the translation that will be copied over). + * @return The content state that the copied translation should have. 'New' indicates that the translation + * should not copied. */ - public @Nullable - ContentState getExpectedContentState( boolean match, HCopyTransOptions.ConditionRuleAction action, - ContentState currentState ) + static + ContentState determineContentStateFromRuleList(List pairs, + boolean requireTranslationReview, ContentState matchingTargetState) { - if( currentState == null ) - { - return null; - } - else if( !match ) + assert matchingTargetState == Translated || matchingTargetState == Approved; + return determineContentStateFromMatchRules(pairs, requireTranslationReview ? matchingTargetState : Translated); + } + + /** + * Determines the content state that a copied translation should have. + * + * @param contextMatches Indicates if there is a context match between the match and copy-target text flows. + * @param projectMatches Indicates if there is a project match between the match and copy-target text flows. + * @param docIdMatches Indicates if there is a doc Id match between the match and copy-target text flows. + * @param options The copy trans options that are effective. + * @param requireTranslationReview Whether the project to copy the translation to requires translations to be reviewed. + * @param matchingTargetState he initial state of the matching translation (the translation that will be copied over). + * @return The content state that the copied translation should have. 'New' indicates that the translation + * should not copied. + */ + static + ContentState determineContentState(boolean contextMatches, boolean projectMatches, boolean docIdMatches, + HCopyTransOptions options, boolean requireTranslationReview, ContentState matchingTargetState) + { + List rules = + ImmutableList.of(new ResultActionPair(contextMatches, options.getContextMismatchAction()), + new ResultActionPair(projectMatches, options.getProjectMismatchAction()), + new ResultActionPair(docIdMatches, options.getDocIdMismatchAction())); + + return determineContentStateFromRuleList(rules, requireTranslationReview, matchingTargetState); + } + + /** + * Indicates if a copied translation should be rejected. + * + * @param match The result of a match evaluating condition. + * @param action The selected action to take based on the result of the condition evaluation. + * @return True, if the translation should be outright rejected based on the evaluated condition. + */ + static boolean shouldReject(boolean match, HCopyTransOptions.ConditionRuleAction action ) + { + if( !match && action == REJECT ) { - if( action == DOWNGRADE_TO_FUZZY ) - { - return NeedReview; - } - else if( action == REJECT ) - { - return null; - } + return true; } - return currentState; + return false; + } + + /** + * Indicates if a copied translation should be downgraded to fuzzy/ + * + * @param match The result of a match evaluating condition. + * @param action The selected action to take based on the result of the condition evaluation. + * @return True, if the translation should be downgraded to fuzzy based on the evaluated condition. + */ + static boolean shouldDowngradeToFuzzy(boolean match, HCopyTransOptions.ConditionRuleAction action) + { + return !match && action == DOWNGRADE_TO_FUZZY; } @Override @@ -461,7 +499,7 @@ private void copyTransForLocale(HDocument document, HLocale locale, HCopyTransOp */ private static boolean shouldOverwrite(HTextFlowTarget currentlyStored, ContentState matchState) { - if( matchState == null ) + if( matchState == null || matchState == New ) { return false; } @@ -486,4 +524,39 @@ else if( currentlyStored.getState() == New ) } return true; } + + /** + * Internal pair implementation. + * TODO: Apache commons collections has a Pair class which could be used here + */ + @Getter + @AllArgsConstructor + private static class Pair + { + final private T1 left; + final private T2 right; + } + + /** + * Internal concrete pair implementation. + * Holds the result of a rule evaluation in the form of a boolean, and the corresponding action + * to be taken for the result. + */ + static final class ResultActionPair extends Pair + { + public ResultActionPair(Boolean result, HCopyTransOptions.ConditionRuleAction action) + { + super(result, action); + } + + public boolean getMatchResult() + { + return super.getLeft(); + } + + public HCopyTransOptions.ConditionRuleAction getRuleAction() + { + return super.getRight(); + } + } } diff --git a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java index 282742dd35..a9de304ee7 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java @@ -20,12 +20,16 @@ */ package org.zanata.service.impl; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.dbunit.operation.DatabaseOperation; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.jboss.seam.security.management.JpaIdentityStore; import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; @@ -50,6 +54,9 @@ import org.zanata.seam.SeamAutowire; import org.zanata.service.CopyTransService; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; + import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.Setter; @@ -57,14 +64,18 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.nullValue; import static org.zanata.common.ContentState.Approved; import static org.zanata.common.ContentState.NeedReview; +import static org.zanata.common.ContentState.New; +import static org.zanata.common.ContentState.Rejected; import static org.zanata.common.ContentState.Translated; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.DOWNGRADE_TO_FUZZY; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.IGNORE; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.REJECT; +import static org.zanata.service.impl.CopyTransServiceImpl.ResultActionPair; +import static org.zanata.service.impl.CopyTransServiceImpl.shouldDowngradeToFuzzy; +import static org.zanata.service.impl.CopyTransServiceImpl.shouldReject; /** * @author Carlos Munoz camunoz@redhat.com @@ -73,6 +84,7 @@ public class CopyTransServiceImplTest extends ZanataDbunitJpaTest { private SeamAutowire seam = SeamAutowire.instance(); + private final List validTranslatedStates = ImmutableList.of(Translated, Approved); @Override protected void prepareDBUnitOperations() @@ -100,7 +112,8 @@ public void initializeSeam() @Test(enabled = false) public void individualTest() { - this.testCopyTrans(new CopyTransExecution(REJECT, IGNORE, DOWNGRADE_TO_FUZZY, true, true, true, true).expectTransState(Approved)); + + this.testCopyTrans(new CopyTransExecution(REJECT, IGNORE, DOWNGRADE_TO_FUZZY, false, true, false, false, Approved).expectTransState(NeedReview)); } @DataProvider(name = "CopyTrans") @@ -119,36 +132,171 @@ public Object[][] createCopyTransTestParams() } @Test - public void testExpectedState() + public void basicDetermineContentState() + { + // An empty rule list should not change the state + for( ContentState state : validTranslatedStates) + { + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), true, state), + is(state)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), false, state), + is(Translated)); + } + } + + @Test + public void contentStateWithIgnoreRule() { - CopyTransServiceImpl copyTransService = seam.autowire(CopyTransServiceImpl.class); - - // When the current context (last argument) is null, it should always return null - assertThat(copyTransService.getExpectedContentState(true, DOWNGRADE_TO_FUZZY, null), nullValue()); - - assertThat(copyTransService.getExpectedContentState(true, REJECT, Approved), is(Approved)); - assertThat(copyTransService.getExpectedContentState(false, REJECT, Approved), nullValue()); - assertThat(copyTransService.getExpectedContentState(true, DOWNGRADE_TO_FUZZY, Approved), is(Approved)); - assertThat(copyTransService.getExpectedContentState(false, DOWNGRADE_TO_FUZZY, Approved), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(true, IGNORE, Approved), is(Approved)); - assertThat(copyTransService.getExpectedContentState(false, IGNORE, Approved), is(Approved)); - - assertThat(copyTransService.getExpectedContentState(true, REJECT, NeedReview), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(false, REJECT, NeedReview), nullValue()); - assertThat(copyTransService.getExpectedContentState(true, DOWNGRADE_TO_FUZZY, NeedReview), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(false, DOWNGRADE_TO_FUZZY, NeedReview), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(true, IGNORE, NeedReview), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(false, IGNORE, NeedReview), is(NeedReview)); - - assertThat(copyTransService.getExpectedContentState(true, REJECT, Translated), is(Translated)); - assertThat(copyTransService.getExpectedContentState(false, REJECT, Translated), nullValue()); - assertThat(copyTransService.getExpectedContentState(true, DOWNGRADE_TO_FUZZY, Translated), is(Translated)); - assertThat(copyTransService.getExpectedContentState(false, DOWNGRADE_TO_FUZZY, Translated), is(NeedReview)); - assertThat(copyTransService.getExpectedContentState(true, IGNORE, Translated), is(Translated)); - assertThat(copyTransService.getExpectedContentState(false, IGNORE, Translated), is(Translated)); + // If the rule is IGNORE, the state should not change no matter what the result is + for( ContentState state : validTranslatedStates ) + { + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, IGNORE) ), true, state), + is(state)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, IGNORE) ), true, state), + is(state)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, IGNORE) ), false, state), + is(Translated)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, IGNORE) ), false, state), + is(Translated)); + } } - @Test(dataProvider = "CopyTrans", dependsOnMethods = "testExpectedState") + @Test + public void contentStateWithRejectRule() + { + // If the rule is Reject, then the match should be rejected only when the evaluation fails + for( ContentState state : validTranslatedStates ) + { + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, REJECT) ), true, state), + is(state)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, REJECT) ), true, state), + is(New)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, REJECT) ), false, state), + is(Translated)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, REJECT) ), false, state), + is(New)); + } + } + + @Test + public void contentStateWithDowngradeRule() + { + // If the rule is downgrade, then the match should be downgraded when the evaluation fails + for( ContentState state : validTranslatedStates) + { + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, DOWNGRADE_TO_FUZZY) ), true, state), + is(state)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, DOWNGRADE_TO_FUZZY) ), true, state), + is(NeedReview)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, DOWNGRADE_TO_FUZZY) ), false, state), + is(Translated)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, DOWNGRADE_TO_FUZZY) ), false, state), + is(NeedReview)); + } + } + + @Test + public void failedRejectionRule() + { + // A single rejection should reject the whole translation no matter what the other rules say + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, DOWNGRADE_TO_FUZZY), + new ResultActionPair(false, REJECT)), + true, Translated), + is(New)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(true, IGNORE), + new ResultActionPair(false, REJECT)), + false, Translated), + is(New)); + + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, REJECT), + new ResultActionPair(false, DOWNGRADE_TO_FUZZY)), + true, Translated), + is(New)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, REJECT), + new ResultActionPair(true, IGNORE)), + false, Translated), + is(New)); + } + + @Test + public void failedDowngradeRule() + { + // A failed Downgrade rule should cause the content state to be fuzzy in all cases, except if a rejection is encountered + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, DOWNGRADE_TO_FUZZY), + new ResultActionPair(true, REJECT)), + true, Translated), + is(NeedReview)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, DOWNGRADE_TO_FUZZY), + new ResultActionPair(true, REJECT)), + false, Translated), + is(NeedReview)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, DOWNGRADE_TO_FUZZY), + new ResultActionPair(false, IGNORE)), + true, Approved), + is(NeedReview)); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( + Lists.newArrayList( + new ResultActionPair(false, DOWNGRADE_TO_FUZZY), + new ResultActionPair(false, IGNORE)), + true, Approved), + is(NeedReview)); + } + + @Test + public void determineContentStateFromRuleListBasics() + { + // Tests the expected content state when approval is/is not required, and NO rules are evaluated + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Translated), + is(Translated) ); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Translated), + is(Translated) ); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Approved), + is(Approved) ); + assertThat( + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Approved), + is(Translated) ); + } + + @Test(dataProvider = "CopyTrans") public void testCopyTrans(CopyTransExecution execution) { // Prepare Execution @@ -169,6 +317,18 @@ public void testCopyTrans(CopyTransExecution execution) // Set require translation review projectIteration.setRequireTranslationReview(execution.requireTranslationReview); + // Change all targets to have the execution's match state + for( HDocument doc : projectIteration.getDocuments().values() ) + { + for( HTextFlow tf : doc.getAllTextFlows().values() ) + { + for( HTextFlowTarget tft : tf.getTargets().values() ) + { + tft.setState( execution.matchState ); + } + } + } + // Create the document HDocument doc = new HDocument(); doc.setContentType(ContentType.TextPlain); @@ -227,7 +387,7 @@ public void testCopyTrans(CopyTransExecution execution) throw new AssertionError("Expected untranslated text flow but got state " + target.getState()); } } - else if( execution.getExpectedTranslationState() != null ) + else if( execution.getExpectedTranslationState() != New ) { if( target == null ) { @@ -270,13 +430,13 @@ public void ignoreTranslationsFromObsoleteProjectAndVersion() throws Exception projectDAO.makePersistent(project); // Run the copy trans scenario (very liberal, but nothing should be translated) - CopyTransExecution execution = new CopyTransExecution(IGNORE, IGNORE, IGNORE, true, true, true, true) + CopyTransExecution execution = new CopyTransExecution(IGNORE, IGNORE, IGNORE, true, true, true, true, Approved) .expectUntranslated(); testCopyTrans(execution); } @Test - public void ignoreTranslationsFromObsoleteDocuments() throws Exception + public void reuseTranslationsFromObsoleteDocuments() throws Exception { ProjectIterationDAO projectIterationDAO = seam.autowire(ProjectIterationDAO.class); DocumentDAO documentDAO = seam.autowire(DocumentDAO.class); @@ -301,19 +461,14 @@ public void ignoreTranslationsFromObsoleteDocuments() throws Exception } // Run the copy trans scenario (very liberal, but nothing should be translated) - CopyTransExecution execution = new CopyTransExecution(IGNORE, IGNORE, IGNORE, true, true, true, true) - .expectUntranslated(); + CopyTransExecution execution = new CopyTransExecution(IGNORE, IGNORE, IGNORE, true, true, true, true, Approved) + .expectTransState(Approved); testCopyTrans(execution); } private ContentState getExpectedContentState( CopyTransExecution execution ) { - ContentState expectedContentState = Translated; - // our test data has content state Approved - if (execution.getRequireTranslationReview() && execution.getContextMatches() && execution.getDocumentMatches() && execution.getProjectMatches()) - { - expectedContentState = Approved; - } + ContentState expectedContentState = execution.getRequireTranslationReview() ? Approved : Translated; expectedContentState = getExpectedContentState(execution.getContextMatches(), execution.getContextMismatchAction(), expectedContentState); expectedContentState = getExpectedContentState(execution.getProjectMatches(), execution.getProjectMismatchAction(), expectedContentState); @@ -321,24 +476,26 @@ private ContentState getExpectedContentState( CopyTransExecution execution ) return expectedContentState; } - private ContentState getExpectedContentState( boolean match, ConditionRuleAction action, ContentState currentState ) + public static + ContentState getExpectedContentState( boolean match, HCopyTransOptions.ConditionRuleAction action, + ContentState currentState ) { - if( currentState == null ) + if( currentState == New ) { - return null; + return currentState; } - else if( !match ) + else if( shouldReject(match, action) ) { - if( action == DOWNGRADE_TO_FUZZY ) - { - return ContentState.NeedReview; - } - else if( action == REJECT ) - { - return null; - } + return New; + } + else if (shouldDowngradeToFuzzy(match, action)) + { + return NeedReview; + } + else + { + return currentState; } - return currentState; } private Set generateAllExecutions() @@ -350,16 +507,17 @@ private Set generateAllExecutions() Arrays.asList(true, false), Arrays.asList(true, false), Arrays.asList(true, false), - Arrays.asList(true, false)); + Arrays.asList(true, false), + Arrays.asList(Translated, Approved)); for( Object[] params : paramsSet ) { CopyTransExecution exec = new CopyTransExecution( (ConditionRuleAction)params[0], (ConditionRuleAction)params[1], (ConditionRuleAction)params[2], - (Boolean)params[3], (Boolean)params[4], (Boolean)params[5], (Boolean)params[6]); + (Boolean)params[3], (Boolean)params[4], (Boolean)params[5], (Boolean)params[6], (ContentState)params[7]); ContentState expectedContentState = this.getExpectedContentState(exec); - if( expectedContentState == null ) + if( expectedContentState == New ) { exec.expectUntranslated(); } @@ -412,8 +570,7 @@ private Set cartesianProduct( Iterable ... colls ) @Getter @Setter - @EqualsAndHashCode(of = {"contextMismatchAction", "projectMismatchAction", "documentMismatchAction", - "contextMatches", "projectMatches", "documentMatches"}) + @EqualsAndHashCode @ToString private static class CopyTransExecution implements Cloneable { @@ -427,10 +584,11 @@ private static class CopyTransExecution implements Cloneable private ContentState expectedTranslationState; private boolean expectUntranslated; private String[] expectedContents; + public ContentState matchState; private CopyTransExecution(ConditionRuleAction contextMismatchAction, ConditionRuleAction projectMismatchAction, ConditionRuleAction documentMismatchAction, Boolean contextMatches, Boolean projectMatches, - Boolean documentMatches, Boolean requireTranslationReview) + Boolean documentMatches, Boolean requireTranslationReview, ContentState matchState) { this.contextMismatchAction = contextMismatchAction; this.projectMismatchAction = projectMismatchAction; @@ -439,6 +597,7 @@ private CopyTransExecution(ConditionRuleAction contextMismatchAction, ConditionR this.projectMatches = projectMatches; this.documentMatches = documentMatches; this.requireTranslationReview = requireTranslationReview; + this.matchState = matchState; } @Override @@ -450,13 +609,13 @@ public Object clone() throws CloneNotSupportedException public CopyTransExecution expectTransState( ContentState state ) { this.expectedTranslationState = state; - this.expectUntranslated = false; + this.expectUntranslated = state == New; return this; } public CopyTransExecution expectUntranslated() { - this.expectedTranslationState = null; + this.expectedTranslationState = New; this.expectUntranslated = true; return this; } From 28321027a6907da2ed059d2391b2997081921e36 Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 12:44:21 +1000 Subject: [PATCH 06/39] Rename ResultActionPair to MatchRulePair. --- .../service/impl/CopyTransServiceImpl.java | 20 ++--- .../impl/CopyTransServiceImplTest.java | 73 +++++++++---------- 2 files changed, 44 insertions(+), 49 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java index a996f8eac9..d22298257b 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java @@ -297,20 +297,20 @@ private int copyTransPass( HDocument document, HLocale locale, boolean checkCont * Determines the content state for a translation given a list of rules and their evaluation result, and * the initial state that it was copied as. * - * @param pairs List of evaluated rules and their result. + * @param pairs List of evaluated rules and the match result. * @param initialState The initial content state of the translation that the content was copied from. * @return The content state that the copied translation should have. 'New' indicates that the translation * should not copied. */ private static - ContentState determineContentStateFromMatchRules(List pairs, ContentState initialState) + ContentState determineContentStateFromMatchRules(List pairs, ContentState initialState) { if( pairs.isEmpty() ) { return initialState; } - ResultActionPair p = pairs.get(0); + MatchRulePair p = pairs.get(0); if( shouldReject(p.getMatchResult(), p.getRuleAction()) ) { return New; @@ -335,7 +335,7 @@ else if( shouldDowngradeToFuzzy(p.getMatchResult(), p.getRuleAction()) ) * should not copied. */ static - ContentState determineContentStateFromRuleList(List pairs, + ContentState determineContentStateFromRuleList(List pairs, boolean requireTranslationReview, ContentState matchingTargetState) { assert matchingTargetState == Translated || matchingTargetState == Approved; @@ -359,9 +359,9 @@ ContentState determineContentState(boolean contextMatches, boolean projectMatche HCopyTransOptions options, boolean requireTranslationReview, ContentState matchingTargetState) { List rules = - ImmutableList.of(new ResultActionPair(contextMatches, options.getContextMismatchAction()), - new ResultActionPair(projectMatches, options.getProjectMismatchAction()), - new ResultActionPair(docIdMatches, options.getDocIdMismatchAction())); + ImmutableList.of(new MatchRulePair(contextMatches, options.getContextMismatchAction()), + new MatchRulePair(projectMatches, options.getProjectMismatchAction()), + new MatchRulePair(docIdMatches, options.getDocIdMismatchAction())); return determineContentStateFromRuleList(rules, requireTranslationReview, matchingTargetState); } @@ -476,7 +476,7 @@ private void copyTransForDocument(HDocument document, HCopyTransOptions options, if( processHandle != null ) { - processHandle.setDocumentsProcessed( processHandle.getDocumentsProcessed() + 1 ); + processHandle.setDocumentsProcessed(processHandle.getDocumentsProcessed() + 1); } log.info("copyTrans finished: document \"{0}\"", document.getDocId()); } @@ -542,9 +542,9 @@ private static class Pair * Holds the result of a rule evaluation in the form of a boolean, and the corresponding action * to be taken for the result. */ - static final class ResultActionPair extends Pair + static final class MatchRulePair extends Pair { - public ResultActionPair(Boolean result, HCopyTransOptions.ConditionRuleAction action) + public MatchRulePair(Boolean result, HCopyTransOptions.ConditionRuleAction action) { super(result, action); } diff --git a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java index a9de304ee7..1b625e6d09 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java @@ -20,7 +20,6 @@ */ package org.zanata.service.impl; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashSet; @@ -28,8 +27,6 @@ import java.util.Set; import org.dbunit.operation.DatabaseOperation; -import org.hamcrest.MatcherAssert; -import org.hamcrest.Matchers; import org.jboss.seam.security.management.JpaIdentityStore; import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; @@ -67,13 +64,11 @@ import static org.zanata.common.ContentState.Approved; import static org.zanata.common.ContentState.NeedReview; import static org.zanata.common.ContentState.New; -import static org.zanata.common.ContentState.Rejected; import static org.zanata.common.ContentState.Translated; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.DOWNGRADE_TO_FUZZY; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.IGNORE; import static org.zanata.model.HCopyTransOptions.ConditionRuleAction.REJECT; -import static org.zanata.service.impl.CopyTransServiceImpl.ResultActionPair; import static org.zanata.service.impl.CopyTransServiceImpl.shouldDowngradeToFuzzy; import static org.zanata.service.impl.CopyTransServiceImpl.shouldReject; @@ -138,10 +133,10 @@ public void basicDetermineContentState() for( ContentState state : validTranslatedStates) { assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), true, state), is(state)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList(), false, state), is(Translated)); } } @@ -153,16 +148,16 @@ public void contentStateWithIgnoreRule() for( ContentState state : validTranslatedStates ) { assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, IGNORE) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, IGNORE) ), true, state), is(state)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, IGNORE) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, IGNORE) ), true, state), is(state)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, IGNORE) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, IGNORE) ), false, state), is(Translated)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, IGNORE) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, IGNORE) ), false, state), is(Translated)); } } @@ -174,16 +169,16 @@ public void contentStateWithRejectRule() for( ContentState state : validTranslatedStates ) { assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, REJECT) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, REJECT) ), true, state), is(state)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, REJECT) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, REJECT) ), true, state), is(New)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, REJECT) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, REJECT) ), false, state), is(Translated)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, REJECT) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, REJECT) ), false, state), is(New)); } } @@ -195,16 +190,16 @@ public void contentStateWithDowngradeRule() for( ContentState state : validTranslatedStates) { assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, DOWNGRADE_TO_FUZZY) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, DOWNGRADE_TO_FUZZY) ), true, state), is(state)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, DOWNGRADE_TO_FUZZY) ), true, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY) ), true, state), is(NeedReview)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(true, DOWNGRADE_TO_FUZZY) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(true, DOWNGRADE_TO_FUZZY) ), false, state), is(Translated)); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new ResultActionPair(false, DOWNGRADE_TO_FUZZY) ), false, state), + CopyTransServiceImpl.determineContentStateFromRuleList(Lists.newArrayList( new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY) ), false, state), is(NeedReview)); } } @@ -216,30 +211,30 @@ public void failedRejectionRule() assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, DOWNGRADE_TO_FUZZY), - new ResultActionPair(false, REJECT)), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY), + new CopyTransServiceImpl.MatchRulePair(false, REJECT)), true, Translated), is(New)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(true, IGNORE), - new ResultActionPair(false, REJECT)), + new CopyTransServiceImpl.MatchRulePair(true, IGNORE), + new CopyTransServiceImpl.MatchRulePair(false, REJECT)), false, Translated), is(New)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, REJECT), - new ResultActionPair(false, DOWNGRADE_TO_FUZZY)), + new CopyTransServiceImpl.MatchRulePair(false, REJECT), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY)), true, Translated), is(New)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, REJECT), - new ResultActionPair(true, IGNORE)), + new CopyTransServiceImpl.MatchRulePair(false, REJECT), + new CopyTransServiceImpl.MatchRulePair(true, IGNORE)), false, Translated), is(New)); } @@ -251,29 +246,29 @@ public void failedDowngradeRule() assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, DOWNGRADE_TO_FUZZY), - new ResultActionPair(true, REJECT)), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY), + new CopyTransServiceImpl.MatchRulePair(true, REJECT)), true, Translated), is(NeedReview)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, DOWNGRADE_TO_FUZZY), - new ResultActionPair(true, REJECT)), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY), + new CopyTransServiceImpl.MatchRulePair(true, REJECT)), false, Translated), is(NeedReview)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, DOWNGRADE_TO_FUZZY), - new ResultActionPair(false, IGNORE)), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY), + new CopyTransServiceImpl.MatchRulePair(false, IGNORE)), true, Approved), is(NeedReview)); assertThat( CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList( - new ResultActionPair(false, DOWNGRADE_TO_FUZZY), - new ResultActionPair(false, IGNORE)), + new CopyTransServiceImpl.MatchRulePair(false, DOWNGRADE_TO_FUZZY), + new CopyTransServiceImpl.MatchRulePair(false, IGNORE)), true, Approved), is(NeedReview)); } @@ -283,16 +278,16 @@ public void determineContentStateFromRuleListBasics() { // Tests the expected content state when approval is/is not required, and NO rules are evaluated assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Translated), + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Translated), is(Translated) ); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Translated), + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Translated), is(Translated) ); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Approved), + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), true, Approved), is(Approved) ); assertThat( - CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Approved), + CopyTransServiceImpl.determineContentStateFromRuleList( Lists.newArrayList(), false, Approved), is(Translated) ); } From d1dae05e6c647eb12506773b0cb5bc5b403fc2e4 Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 12:47:34 +1000 Subject: [PATCH 07/39] Code review changes (See below). Refactor method to use ternary operator. Assume a non-nullable argument in copy trans internal method. --- .../org/zanata/service/impl/CopyTransServiceImpl.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java index d22298257b..2fd61f7d31 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java @@ -375,11 +375,7 @@ ContentState determineContentState(boolean contextMatches, boolean projectMatche */ static boolean shouldReject(boolean match, HCopyTransOptions.ConditionRuleAction action ) { - if( !match && action == REJECT ) - { - return true; - } - return false; + return !match && action == REJECT; } /** @@ -499,7 +495,7 @@ private void copyTransForLocale(HDocument document, HLocale locale, HCopyTransOp */ private static boolean shouldOverwrite(HTextFlowTarget currentlyStored, ContentState matchState) { - if( matchState == null || matchState == New ) + if( matchState == New ) { return false; } From 3512081da1f4ba861f77313cc855161d12474248 Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 12:50:11 +1000 Subject: [PATCH 08/39] Refactor internal MatchRulePair class. --- .../service/impl/CopyTransServiceImpl.java | 35 ++++--------------- 1 file changed, 6 insertions(+), 29 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java index 2fd61f7d31..0001e47fac 100644 --- a/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java +++ b/zanata-war/src/main/java/org/zanata/service/impl/CopyTransServiceImpl.java @@ -522,37 +522,14 @@ else if( currentlyStored.getState() == New ) } /** - * Internal pair implementation. - * TODO: Apache commons collections has a Pair class which could be used here - */ - @Getter - @AllArgsConstructor - private static class Pair - { - final private T1 left; - final private T2 right; - } - - /** - * Internal concrete pair implementation. - * Holds the result of a rule evaluation in the form of a boolean, and the corresponding action + * Holds the result of a match evaluation in the form of a boolean, and the corresponding action * to be taken for the result. */ - static final class MatchRulePair extends Pair + @AllArgsConstructor + @Getter + static final class MatchRulePair { - public MatchRulePair(Boolean result, HCopyTransOptions.ConditionRuleAction action) - { - super(result, action); - } - - public boolean getMatchResult() - { - return super.getLeft(); - } - - public HCopyTransOptions.ConditionRuleAction getRuleAction() - { - return super.getRight(); - } + private final Boolean matchResult; + private final HCopyTransOptions.ConditionRuleAction ruleAction; } } From da8e36ad77d9bc2b8aebe681dbd7bbd16d6b7849 Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 12:54:04 +1000 Subject: [PATCH 09/39] Add @SlowTest annotation. For now just a marker interface to identify potentially slow running tests. --- .../src/test/java/org/zanata/SlowTest.java | 37 +++++++++++++++++++ .../impl/CopyTransServiceImplTest.java | 2 + 2 files changed, 39 insertions(+) create mode 100644 zanata-war/src/test/java/org/zanata/SlowTest.java diff --git a/zanata-war/src/test/java/org/zanata/SlowTest.java b/zanata-war/src/test/java/org/zanata/SlowTest.java new file mode 100644 index 0000000000..cb23cf610b --- /dev/null +++ b/zanata-war/src/test/java/org/zanata/SlowTest.java @@ -0,0 +1,37 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ +package org.zanata; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Marker annotation to identify tests that might be potentially slow to run. + * + * @author Carlos Munoz camunoz@redhat.com + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface SlowTest +{ +} diff --git a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java index 1b625e6d09..5baf1763e6 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java @@ -31,6 +31,7 @@ import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import org.zanata.SlowTest; import org.zanata.ZanataDbunitJpaTest; import org.zanata.common.ContentState; import org.zanata.common.ContentType; @@ -76,6 +77,7 @@ * @author Carlos Munoz camunoz@redhat.com */ @Test(groups = { "business-tests" }) +@SlowTest public class CopyTransServiceImplTest extends ZanataDbunitJpaTest { private SeamAutowire seam = SeamAutowire.instance(); From 3d0f8bd97f49b4ed804ee21e5f786c446270ebdc Mon Sep 17 00:00:00 2001 From: "Carlos A. Munoz" Date: Wed, 4 Sep 2013 12:56:40 +1000 Subject: [PATCH 10/39] Change @SlowTest to be a method level annotation. --- zanata-war/src/test/java/org/zanata/SlowTest.java | 2 +- .../java/org/zanata/service/impl/CopyTransServiceImplTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zanata-war/src/test/java/org/zanata/SlowTest.java b/zanata-war/src/test/java/org/zanata/SlowTest.java index cb23cf610b..5eb1895c3c 100644 --- a/zanata-war/src/test/java/org/zanata/SlowTest.java +++ b/zanata-war/src/test/java/org/zanata/SlowTest.java @@ -31,7 +31,7 @@ * @author Carlos Munoz camunoz@redhat.com */ @Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) +@Target(ElementType.METHOD) public @interface SlowTest { } diff --git a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java index 5baf1763e6..fdfcaab615 100644 --- a/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java +++ b/zanata-war/src/test/java/org/zanata/service/impl/CopyTransServiceImplTest.java @@ -77,7 +77,6 @@ * @author Carlos Munoz camunoz@redhat.com */ @Test(groups = { "business-tests" }) -@SlowTest public class CopyTransServiceImplTest extends ZanataDbunitJpaTest { private SeamAutowire seam = SeamAutowire.instance(); @@ -294,6 +293,7 @@ public void determineContentStateFromRuleListBasics() } @Test(dataProvider = "CopyTrans") + @SlowTest public void testCopyTrans(CopyTransExecution execution) { // Prepare Execution From f9e65e7111e365e5ae1ed8db8a236dedf958b25b Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 09:15:53 +1000 Subject: [PATCH 11/39] test cater for randomly selected error message --- .../zanata/feature/security/SecurityFullTest.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/functional-test/src/test/java/org/zanata/feature/security/SecurityFullTest.java b/functional-test/src/test/java/org/zanata/feature/security/SecurityFullTest.java index d32dfb9ec0..b6b5e14ddc 100644 --- a/functional-test/src/test/java/org/zanata/feature/security/SecurityFullTest.java +++ b/functional-test/src/test/java/org/zanata/feature/security/SecurityFullTest.java @@ -116,11 +116,6 @@ public void invalidResetPasswordFieldEntries() @Test public void emptyResetPasswordFieldEntries() { - // Both are valid, but show seemingly at random - List emptyUsernameErrors = new ArrayList(); - emptyUsernameErrors.add("size must be between 3 and 20"); - emptyUsernameErrors.add("must match ^[a-z\\d_]{3,20}$"); - SignInPage signInPage = new BasicWorkFlow().goToHome().clickSignInLink(); ResetPasswordPage resetPasswordPage = signInPage.gotToResetPassword(); resetPasswordPage = resetPasswordPage.clearFields(); @@ -129,8 +124,11 @@ public void emptyResetPasswordFieldEntries() assertThat("Empty email error is displayed", resetPasswordPage.waitForErrors(), hasItem("may not be empty")); - - assertThat(resetPasswordPage.getErrors().get(0), equalTo("may not be empty")); + // All are valid, but may show at random + assertThat(resetPasswordPage.getErrors().get(0), + either(equalTo("size must be between 3 and 20")) + .or(equalTo("may not be empty")) + .or(equalTo("must match ^[a-z\\d_]{3,20}$"))); } From bd2de11a94d61ade3f58ec088793031ed3eee811 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Tue, 13 Aug 2013 10:04:34 +1000 Subject: [PATCH 12/39] Clean up dependencies - Fail on errors from maven-dependency-plugin and maven-enforcer-plugin - Update parent version - Simplify configuration for maven-enforcer-plugin - Exclude unused or conflicting dependencies - Declare dependencies which were used but not declared - Update shrinkwrap-resolver and fix Deployments class - Put boms at the top of server pom to avoid mismatched dependencies --- functional-test/pom.xml | 79 +++- pom.xml | 227 ++++------ zanata-model/pom.xml | 92 +++- zanata-war/pom.xml | 418 ++++++++++-------- .../org/zanata/arquillian/Deployments.java | 43 +- 5 files changed, 504 insertions(+), 355 deletions(-) diff --git a/functional-test/pom.xml b/functional-test/pom.xml index 121d0c3952..08076fca06 100644 --- a/functional-test/pom.xml +++ b/functional-test/pom.xml @@ -68,27 +68,48 @@ org.seleniumhq.selenium - selenium-java + selenium-api + ${selenium.version} + + + org.seleniumhq.selenium + selenium-chrome-driver + ${selenium.version} + + + org.seleniumhq.selenium + selenium-firefox-driver + ${selenium.version} + + + org.seleniumhq.selenium + selenium-htmlunit-driver ${selenium.version} - - org.hamcrest - hamcrest-all - - - junit - junit - - - xml-apis - xml-apis - - - xercesImpl - xerces - + + commons-logging + commons-logging + + + xml-apis + xml-apis + + + org.seleniumhq.selenium + selenium-remote-driver + ${selenium.version} + + + org.seleniumhq.selenium + selenium-support + ${selenium.version} + + + com.google.code.findbugs + jsr305 + com.google.guava guava @@ -107,10 +128,6 @@ junit junit - - log4j - log4j - org.slf4j slf4j-api @@ -164,6 +181,11 @@ ${hibernate.scope} + + org.projectlombok + lombok + + ${jdbc.groupId} ${jdbc.artifactId} @@ -415,6 +437,21 @@ + + maven-dependency-plugin + + true + + com.h2database:h2 + org.hibernate:hibernate-core + org.hibernate:hibernate-envers + org.hibernate:hibernate-entitymanager + org.hibernate:hibernate-infinispan + org.projectlombok:lombok + + + + maven-surefire-plugin diff --git a/pom.xml b/pom.xml index 21970a7656..a043a6e0cc 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ org.zanata zanata-parent - 13 + 14-SNAPSHOT ../parent @@ -55,20 +55,76 @@ - + + + + org.jboss.shrinkwrap + shrinkwrap-bom + 1.1.2 + import + pom + + + + + org.jboss.shrinkwrap.resolver + shrinkwrap-resolver-bom + 2.0.0 + import + pom + + - org.zanata - zanata-common-api - ${zanata.api.version} + org.jboss.arquillian + arquillian-bom + 1.0.3.Final + import + pom + + + + org.jboss.resteasy + resteasy-bom + ${resteasy.version} + pom + import + + + + + org.richfaces + richfaces-bom + ${richfaces.version} + import + pom + + + + + org.jboss.seam + bom + ${seam.version} + pom + import + + + + org.jboss.as + jboss-as-parent + 7.1.3.Final + pom + import + + + org.zanata zanata-common-api ${zanata.api.version} - test-jar - test - + org.zanata @@ -235,11 +291,22 @@ + + net.sf.okapi.steps + okapi-step-tokenization + ${okapi.version} + + + net.sf.okapi.logbind + build-log4j + + + + net.sf.okapi.steps okapi-step-wordcount ${okapi.version} - net.sf.okapi.logbind @@ -326,33 +393,6 @@ 1.0_02.CR6 - - - org.jboss.resteasy - resteasy-bom - ${resteasy.version} - pom - import - - - - - org.richfaces - richfaces-bom - ${richfaces.version} - import - pom - - - - - org.jboss.seam - bom - ${seam.version} - pom - import - - org.jboss.seam jboss-seam @@ -425,42 +465,6 @@ compile - - - - - org.jboss.shrinkwrap.resolver - shrinkwrap-resolver-api - 2.0.0-beta-3 - - - - org.jboss.shrinkwrap.resolver - shrinkwrap-resolver-api-maven - 2.0.0-beta-3 - - - - org.jboss.shrinkwrap.resolver - shrinkwrap-resolver-impl-maven - 2.0.0-beta-3 - - - - - org.jboss.arquillian.protocol - arquillian-protocol-servlet - 1.0.3.Final - - - - - org.jboss.arquillian - arquillian-bom - 1.0.3.Final - import - pom - org.concordion concordion @@ -516,37 +520,16 @@ + + + javax.xml.stream + stax-api + 1.0 + + - - - org.projectlombok - lombok - - - org.slf4j - slf4j-log4j12 - test - - - org.zanata - zanata-common-api - test-jar - test - - - - xom - xom - 1.2.5 - - - - com.sun.jna.* - + + + + org.projectlombok + lombok + + com.sun.jna.* + + + - - - - - - enforce-no-repositories - - enforce - - - jboss-public-repository-group okapi-cloudbees-release - + cobertura-it-maven-plugin-maven2-release jboss-public-repository-group - - - - + + diff --git a/zanata-model/pom.xml b/zanata-model/pom.xml index 9e0a496a65..46d6cd8200 100644 --- a/zanata-model/pom.xml +++ b/zanata-model/pom.xml @@ -28,6 +28,19 @@ + + + maven-dependency-plugin + + true + + junit:junit + org.projectlombok:lombok + org.hibernate:hibernate-search + + + + + okapi-step-tokenization @@ -212,6 +217,11 @@ + + org.codehaus.jackson + jackson-mapper-asl + + org.jboss.seam @@ -224,6 +234,11 @@ hibernate-search + + org.hibernate + hibernate-search-engine + + org.hibernate hibernate-core @@ -235,8 +250,13 @@ - org.hibernate - hibernate-entitymanager + org.hibernate.javax.persistence + hibernate-jpa-2.0-api + + + + com.google.guava + guava @@ -244,6 +264,11 @@ icu4j + + commons-lang + commons-lang + + javax.validation validation-api @@ -258,6 +283,27 @@ com.google.code.findbugs jsr305 + + org.zanata + zanata-common-api + + + javassist + javassist + + + + + + xom + xom + + + xalan + xalan + + + diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index 1270dab613..7a83ce0eba 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -18,7 +18,6 @@ - beta3.SP12 ${basedir}/src/etc jboss71x true @@ -57,116 +56,117 @@ maven-dependency-plugin + + dependency-properties initialize properties + + true + + javax.xml.bind:jaxb-api + + com.mattbertolini:liquibase-slf4j + com.google.code.findbugs:annotations + quartz:quartz + + org.jboss.shrinkwrap.resolver:shrinkwrap-resolver-api + + + + antlr:antlr + com.google.code.findbugs:jsr305 + com.google.guava:guava-gwt + com.ibm.icu:icu4j + com.sun.faces:jsf-impl + commons-codec:commons-codec + commons-collections:commons-collections + commons-lang:commons-lang + javax.el:el-api + javax.servlet.jsp:jsp-api + javax.servlet:javax.servlet-api + mysql:mysql-connector-java + org.apache.solr:solr-core + org.apache.solr:solr-solrj + org.codehaus.jackson:jackson-xc + org.drools:drools-compiler + org.hibernate:hibernate-ehcache + org.hibernate:hibernate-search-analyzers + org.hibernate:hibernate-search + org.hibernate:hibernate-testing + org.htmlparser:htmlparser + org.jboss.arquillian.junit:arquillian-junit-container + org.jboss.arquillian.protocol:arquillian-protocol-servlet + org.jboss.as:jboss-as-arquillian-container-managed + org.jboss.resteasy:resteasy-jackson-provider + org.jboss.seam:jboss-seam-debug + org.jboss.seam:jboss-seam-mail + org.liquibase.ext:modify-column + org.openxri:openxri-client + org.openxri:openxri-syntax + org.richfaces.core:richfaces-core-impl + org.tuckey:urlrewritefilter + org.zanata:zanata-rest-client + + - org.apache.maven.plugins maven-enforcer-plugin - - - enforce-ban-duplicate-classes - - enforce - - - - false - + + - - org.apache.commons.collections.* - - - antlr.* - com.sun.activation.* - com.sun.xml.* - com.sun.istack.* - hsqlServlet - javassist.* - javax.activation.* - javax.annotation.* - javax.ejb.* - javax.interceptor.* - javax.persistence.* - javax.transaction.* - javax.xml.XMLConstants - javax.xml.bind.* - javax.xml.namespace.* - javax.xml.stream.* - org.apache.commons.logging.* - org.apache.log4j.* - org.dom4j.* - org.hsqldb.* - org.jboss.crypto.* - org.jboss.deployment.* - org.jboss.logging.Logger - org.jboss.Main* - org.jboss.mx.* - org.jboss.security.* - org.jboss.system.* - org.jboss.Version - org.quartz.* + + com.google.common.* com.google.gwt.* com.google.web.bindery.* com.ibm.icu.* com.steadystate.css.* + javax.annotation.* + javax.servlet.* javax.servlet.jsp.* javax.validation.ConstraintViolationException_CustomFieldSerializer + javax.xml.* org.apache.commons.beanutils.* org.apache.commons.codec.* + org.apache.commons.collections.* org.apache.commons.io.* org.apache.commons.lang.* + org.apache.commons.logging.* + org.apache.html.* org.apache.http.* org.apache.james.mime4j.* org.apache.regexp.* - org.cyberneko.html.* - org.hibernate.validator.* - org.w3c.css.sac.* - - - javax.servlet.* - javax.xml.* - org.apache.html.* org.apache.wml.* org.apache.xerces.* + org.apache.xml.* org.apache.xmlcommons.Version - org.apache.xml.serialize.* + org.cyberneko.html.* + org.hibernate.validator.* + org.w3c.css.sac.* org.w3c.dom.* org.xml.sax.* + + + org.osgi.util.* + + com.sun.tools.* + sun.* - - - - - enforce-arquillian-jboss-location-present - - enforce - - - - - arquillian.jboss.home - You will need to set the arquillian.jboss.home property to run integration tests - - - false - - - + + arquillian.jboss.home + You must set the arquillian.jboss.home property to run integration tests + + + @@ -1112,6 +1112,10 @@ javassist javassist + + jcip-annotations + net.jcip + @@ -1145,15 +1149,24 @@ org.codehaus.jackson jackson-mapper-asl - - org.codehaus.jackson - jackson-jaxrs - org.codehaus.jackson jackson-xc + + org.jboss.spec.javax.faces + jboss-jsf-api_2.1_spec + + + org.jboss.spec.javax.transaction + jboss-transaction-api_1.1_spec + + + org.jboss + jboss-dmr + test + @@ -1175,6 +1188,10 @@ + + org.richfaces.ui + richfaces-components-api + org.richfaces.ui richfaces-components-ui @@ -1189,7 +1206,6 @@ com.sun.faces jsf-impl - 2.1.13 @@ -1228,14 +1244,14 @@ org.hibernate hibernate-testing test + + + tools + com.sun + + - - javax.transaction - jta - provided - - net.sf.ehcache ehcache-core @@ -1260,10 +1276,6 @@ net.sf.okapi.filters okapi-filter-plaintext - - net.sf.okapi.filters - okapi-filter-tmx - net.sf.okapi @@ -1399,11 +1411,18 @@ hibernate-search-analyzers - - commons-logging - commons-logging - provided - + + org.hibernate.javax.persistence + hibernate-jpa-2.0-api + + + org.hibernate + hibernate-search-engine + + + org.hibernate + hibernate-search-orm + javax.el @@ -1448,28 +1467,6 @@ test - - - - javax.annotation jsr250-api @@ -1481,13 +1478,7 @@ stax-api provided - + @@ -1507,45 +1498,11 @@ provided - - - - org.projectlombok @@ -1553,6 +1510,23 @@ provided + + org.jboss.shrinkwrap + shrinkwrap-api + test + + + + org.jboss.shrinkwrap.resolver + shrinkwrap-resolver-api-maven + test + + + org.jboss.shrinkwrap.resolver + shrinkwrap-resolver-api + test + + + + org.jboss.arquillian.config + arquillian-config-api + test + + + org.jboss.arquillian.container + arquillian-container-test-api + test + + + org.jboss.arquillian.container + arquillian-container-test-impl-base + test + + + org.jboss.arquillian.container + arquillian-container-spi + test + + + org.jboss.arquillian.core + arquillian-core-spi + test + + + org.jboss.arquillian.core + arquillian-core-api + test + org.jboss.arquillian.junit arquillian-junit-container test + + org.jboss.arquillian.junit + arquillian-junit-core + test + + + org.jboss.arquillian.test + arquillian-test-api + test + + + org.jboss.arquillian.test + arquillian-test-spi + test + org.jboss.as jboss-as-arquillian-container-managed 7.1.3.Final test + + + jboss-logmanager + org.jboss.logmanager + + + log4j-jboss-logmanager + org.jboss.logmanager + + + + + org.jboss.as + jboss-as-arquillian-common + test + + + + log4j-jboss-logmanager + + + org.jboss.logmanager + + + + + + org.jboss.as + jboss-as-controller-client + test + + + org.jboss.as + jboss-as-controller + test @@ -1740,11 +1794,21 @@ - - org.openid4java - openid4java-nodeps - 0.9.5 - + + org.openid4java + openid4java-nodeps + 0.9.5 + + + + commons-logging + + + commons-logging + + + + org.openxri @@ -1763,6 +1827,10 @@ org.slf4j slf4j-jcl + + commons-logging + commons-logging + @@ -1775,6 +1843,10 @@ org.slf4j slf4j-jcl + + commons-logging + commons-logging + @@ -1805,10 +1877,6 @@ joda-time joda-time - - commons-beanutils - commons-beanutils - com.beust jcommander @@ -1841,11 +1909,13 @@ xom xom + + + xalan + xalan + + - - org.functionaljava - functionaljava - diff --git a/zanata-war/src/test/java/org/zanata/arquillian/Deployments.java b/zanata-war/src/test/java/org/zanata/arquillian/Deployments.java index 058d621009..97f8832b9f 100644 --- a/zanata-war/src/test/java/org/zanata/arquillian/Deployments.java +++ b/zanata-war/src/test/java/org/zanata/arquillian/Deployments.java @@ -21,6 +21,9 @@ package org.zanata.arquillian; import java.io.File; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import org.apache.commons.io.FileUtils; import org.jboss.arquillian.container.test.api.Deployment; @@ -34,7 +37,17 @@ import org.jboss.shrinkwrap.api.exporter.ZipExporter; import org.jboss.shrinkwrap.api.spec.WebArchive; import org.jboss.shrinkwrap.resolver.api.maven.Maven; +import org.jboss.shrinkwrap.resolver.api.maven.coordinate.MavenDependency; +import org.jboss.shrinkwrap.resolver.api.maven.filter.MavenResolutionFilter; +import org.jboss.shrinkwrap.resolver.api.maven.filter.RejectDependenciesFilter; +import org.jboss.shrinkwrap.resolver.api.maven.strategy.DefaultTransitiveExclusionPolicy; +import org.jboss.shrinkwrap.resolver.api.maven.strategy.MavenResolutionStrategy; import org.jboss.shrinkwrap.resolver.api.maven.strategy.RejectDependenciesStrategy; +import org.jboss.shrinkwrap.resolver.api.maven.strategy.TransitiveExclusionPolicy; +import org.jboss.shrinkwrap.resolver.api.maven.strategy.TransitiveStrategy; +import org.openxri.resolve.ResolveChain; + +import com.google.common.collect.ImmutableList; /** * Contains Suite-wide deployments to avoid having to deploy the same package for @@ -46,18 +59,32 @@ public class Deployments { public static final String DEPLOYMENT_NAME = "zanata-tests"; - @Deployment(name = "zanata.war") - public static Archive createDeployment() + public static void main(String[] args) { + System.out.println("resolving dependencies:"); + List depList = Arrays.asList(runtimeDependenciesFromPom()); + Collections.sort(depList); + System.out.println(depList); + System.out.println("dependency count: "+depList.size()); + } - WebArchive archive = ShrinkWrap.create(WebArchive.class, DEPLOYMENT_NAME + ".war"); - archive.addAsLibraries(Maven.resolver() + private static File[] runtimeDependenciesFromPom() + { + return Maven.resolver() .loadPomFromFile("pom.xml") - .importRuntimeDependencies( - // Reject some not needed dependencies - new RejectDependenciesStrategy("net.bull.javamelody:javamelody-core") + .importRuntimeDependencies() + .resolve().using( + // JavaMelody's ServletFilter/Listener interfere with test deployments + new RejectDependenciesStrategy(false, "net.bull.javamelody:javamelody-core") ) - .asFile()); + .asFile(); + } + + @Deployment(name = "zanata.war") + public static Archive createDeployment() + { + WebArchive archive = ShrinkWrap.create(WebArchive.class, DEPLOYMENT_NAME + ".war"); + archive.addAsLibraries(runtimeDependenciesFromPom()); // Test dependencies archive.addAsLibraries(Maven.resolver().loadPomFromFile("pom.xml").resolve("org.hibernate:hibernate-testing").withoutTransitivity().asFile()); From 507efba4429c3c8a74657bbdc856255b37a2f7e7 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 13:08:21 +1000 Subject: [PATCH 13/39] Activate profiles with properties --- zanata-war/pom.xml | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index 7a83ce0eba..e41690bc58 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -367,8 +367,8 @@ none:none org.testng:testng once - -Xmx1024m -XX:MaxPermSize=512m -XX:+UseConcMarkSweepGC - -XX:+CMSClassUnloadingEnabled -XX:+HeapDumpOnOutOfMemoryError + -Xmx1024m -XX:MaxPermSize=512m -XX:+UseConcMarkSweepGC + -XX:+CMSClassUnloadingEnabled -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=${project.build.directory} -Dsun.lang.ClassLoader.allowArraySyntax=true src/test/resources/AllNonContainerTests.tng.xml @@ -803,18 +803,28 @@ - - - nogwt - - true - - + + + nogwt + + + nogwt + + + + true + + chrome + + + chrome + + org.zanata.webtrans.ApplicationSafari @@ -836,6 +846,11 @@ firefox + + + firefox + + org.zanata.webtrans.ApplicationGecko18 @@ -857,6 +872,11 @@ chromefirefox + + + chromefirefox + + org.zanata.webtrans.ApplicationChromeFirefox From 9cac6de6b93c573bb1f837f8b0a7ecdd9cf93002 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 13:09:03 +1000 Subject: [PATCH 14/39] Check arquillian.jboss.home property during pre-integration-test --- zanata-war/pom.xml | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index e41690bc58..1a90711fb9 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -116,6 +116,23 @@ maven-enforcer-plugin + + + enforce-arquillian-jboss-home + + enforce + + pre-integration-test + + + + arquillian.jboss.home + You must set the arquillian.jboss.home property to run integration tests + + + + + @@ -161,10 +178,6 @@ sun.* - - arquillian.jboss.home - You must set the arquillian.jboss.home property to run integration tests - From db31b9574893d0a5cee00ab7b2febb9fe34939f8 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 16 Aug 2013 09:47:17 +1000 Subject: [PATCH 15/39] rhbz996398 - short cut key context should change when review comment popup shows --- .../webtrans/client/keys/ShortcutContext.java | 20 +++++--- .../ForceReviewCommentPresenter.java | 47 ++++++++++++++++++- .../client/ui/ReviewCommentInputWidget.java | 18 +++++-- .../client/ui/TranslationHistoryView.java | 2 +- .../view/ForceReviewCommentDisplay.java | 2 + .../client/view/ForceReviewCommentWidget.java | 8 +++- .../ForceReviewCommentPresenterTest.java | 20 +++++++- 7 files changed, 101 insertions(+), 16 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java b/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java index 0ec9321ee4..6c7b00809c 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java @@ -39,32 +39,38 @@ public enum ShortcutContext Application, /** - * Used by {@link SearchResultsPresenter} + * Used by {@link org.zanata.webtrans.client.presenter.SearchResultsPresenter} */ ProjectWideSearch, /** - * Used by {@link TableEditorPresenter} + * Used by {@link org.zanata.webtrans.client.presenter.TranslationPresenter} */ Navigation, /** - * Used by {@link InlineTargetCellEditor} + * Used by {@link org.zanata.webtrans.client.presenter.TargetContentsPresenter} */ Edit, /** - * Used by {@link TransMemoryPresenter} + * Used by {@link org.zanata.webtrans.client.presenter.TransMemoryPresenter} */ TM, /** - * Used by {@link GlossaryPresenter} + * Used by {@link org.zanata.webtrans.client.presenter.GlossaryPresenter} */ Glossary, /** - * Used by {@link WorkspaceUsersPresenter} + * Used by {@link org.zanata.webtrans.client.presenter.WorkspaceUsersPresenter} */ - Chat; + Chat, + + /** + * Used by all popup + */ + Popup + } \ No newline at end of file diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java index 1cd240a64d..7b7c0de4e6 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java @@ -23,14 +23,21 @@ import org.zanata.webtrans.client.events.CommentBeforeSaveEvent; import org.zanata.webtrans.client.events.CommentBeforeSaveEventHandler; +import org.zanata.webtrans.client.events.KeyShortcutEvent; +import org.zanata.webtrans.client.events.KeyShortcutEventHandler; import org.zanata.webtrans.client.events.NavTransUnitEvent; import org.zanata.webtrans.client.events.TransUnitSaveEvent; +import org.zanata.webtrans.client.keys.KeyShortcut; +import org.zanata.webtrans.client.keys.Keys; +import org.zanata.webtrans.client.keys.ShortcutContext; +import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.AbstractAsyncCallback; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; import org.zanata.webtrans.client.view.ForceReviewCommentDisplay; import org.zanata.webtrans.shared.rpc.AddReviewCommentAction; import org.zanata.webtrans.shared.rpc.AddReviewCommentResult; +import com.google.gwt.event.dom.client.KeyCodes; import com.google.inject.Inject; import net.customware.gwt.presenter.client.EventBus; @@ -43,20 +50,45 @@ public class ForceReviewCommentPresenter extends WidgetPresenter diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java index db81781a40..08dad714f3 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java @@ -95,7 +95,7 @@ public void addCommentToList(ReviewComment comment) @Override public void clearInput() { - commentInput.clearInput(); + commentInput.setText(""); } @Override diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentDisplay.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentDisplay.java index 8ea9328730..e15dfafe13 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentDisplay.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentDisplay.java @@ -36,6 +36,8 @@ public interface ForceReviewCommentDisplay extends WidgetDisplay void center(); + String getComment(); + interface Listener { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java index 0ba29fac27..fc4057a622 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java @@ -66,6 +66,12 @@ public void setListener(Listener listener) @Override public void clearInput() { - inputWidget.clearInput(); + inputWidget.setText(""); + } + + @Override + public String getComment() + { + return inputWidget.getText(); } } diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java index 42e306ecf4..c8e01310f0 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java @@ -24,6 +24,7 @@ import org.hamcrest.Matchers; import org.mockito.Answers; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; @@ -32,6 +33,9 @@ import org.zanata.webtrans.client.events.CommentBeforeSaveEvent; import org.zanata.webtrans.client.events.NavTransUnitEvent; import org.zanata.webtrans.client.events.TransUnitSaveEvent; +import org.zanata.webtrans.client.keys.KeyShortcut; +import org.zanata.webtrans.client.keys.ShortcutContext; +import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; import org.zanata.webtrans.client.view.ForceReviewCommentDisplay; @@ -65,16 +69,26 @@ public class ForceReviewCommentPresenterTest private CommentBeforeSaveEvent commentBeforeSaveEvent; @Mock private TransUnitSaveEvent saveEvent; + @Mock + private KeyShortcutPresenter keyShortcutPresenter; + @Mock + private WebTransMessages messages; + @Captor + private ArgumentCaptor shortcutCapture; @BeforeMethod public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - presenter = new ForceReviewCommentPresenter(display, eventBus, dispatcher, contextHolder); + presenter = new ForceReviewCommentPresenter(display, eventBus, dispatcher, contextHolder, keyShortcutPresenter, messages); verify(display).setListener(presenter); verify(eventBus).addHandler(CommentBeforeSaveEvent.TYPE, presenter); + verify(keyShortcutPresenter).register(shortcutCapture.capture()); + + KeyShortcut keyShortcut = shortcutCapture.getValue(); + assertThat(keyShortcut.getContext(), Matchers.equalTo(ShortcutContext.Popup)); } @Test @@ -83,6 +97,8 @@ public void testOnCommentBeforeSave() throws Exception presenter.onCommentBeforeSave(commentBeforeSaveEvent); verify(display).center(); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, false); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, true); } @Test @@ -108,5 +124,7 @@ public void testAddComment() throws Exception verify(eventBus).fireEvent(saveEvent); verify(eventBus).fireEvent(NavTransUnitEvent.NEXT_ENTRY_EVENT); verify(display).hide(); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, true); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, false); } } From ea5296dfe67c23fcf802ff8ae3e5120fe767a35a Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 3 Sep 2013 11:02:55 +1000 Subject: [PATCH 16/39] rhbz996398 - disable editor shortcut when history popup shows --- .../TranslationHistoryPresenter.java | 27 +++++++++++++++---- .../client/ui/TranslationHistoryDisplay.java | 2 ++ .../client/ui/TranslationHistoryView.java | 10 ++++--- .../TranslationHistoryPresenterTest.java | 26 ++++++++++-------- 4 files changed, 46 insertions(+), 19 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java index b6ccf59353..6f3ba1f5b9 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java @@ -4,13 +4,11 @@ import java.util.Date; import java.util.List; -import net.customware.gwt.presenter.client.EventBus; -import net.customware.gwt.presenter.client.widget.WidgetPresenter; - import org.zanata.webtrans.client.events.CopyDataToEditorEvent; import org.zanata.webtrans.client.events.NotificationEvent; import org.zanata.webtrans.client.events.ReviewCommentEvent; import org.zanata.webtrans.client.events.ReviewCommentEventHandler; +import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.AbstractAsyncCallback; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; @@ -24,7 +22,6 @@ import org.zanata.webtrans.shared.rpc.AddReviewCommentResult; import org.zanata.webtrans.shared.rpc.GetTranslationHistoryAction; import org.zanata.webtrans.shared.rpc.GetTranslationHistoryResult; - import com.allen_sauer.gwt.log.client.Log; import com.google.common.base.Objects; import com.google.common.collect.Lists; @@ -32,6 +29,9 @@ import com.google.inject.Inject; import com.google.inject.Singleton; +import net.customware.gwt.presenter.client.EventBus; +import net.customware.gwt.presenter.client.widget.WidgetPresenter; + /** * @author Patrick Huang pahuang@redhat.com */ @@ -44,12 +44,13 @@ public class TranslationHistoryPresenter extends WidgetPresenter otherEntries, List reviewComments) @@ -154,6 +158,19 @@ public void compareClicked(TransHistoryItem item) } } + private void enableShortcut() + { + keyShortcutPresenter.setContextActive(ShortcutContext.Edit, false); + keyShortcutPresenter.setContextActive(ShortcutContext.Popup, true); + } + + @Override + public void disableShortcut() + { + keyShortcutPresenter.setContextActive(ShortcutContext.Edit, true); + keyShortcutPresenter.setContextActive(ShortcutContext.Popup, false); + } + @Override public boolean isItemInComparison(TransHistoryItem item) { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java index 328bb09768..e5b7c56b82 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java @@ -42,5 +42,7 @@ interface Listener extends ForceReviewCommentDisplay.Listener void compareClicked(TransHistoryItem item); boolean isItemInComparison(TransHistoryItem item); + + void disableShortcut(); } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java index 08dad714f3..2c710079b9 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java @@ -8,12 +8,9 @@ import org.zanata.webtrans.shared.model.TransHistoryItem; import com.google.common.collect.Lists; import com.google.gwt.core.client.GWT; -import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.resources.client.CssResource; import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiField; -import com.google.gwt.uibinder.client.UiHandler; -import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.DialogBox; import com.google.gwt.user.client.ui.HTMLPanel; import com.google.gwt.user.client.ui.TabLayoutPanel; @@ -69,6 +66,13 @@ public void setData(List items) redrawList(); } + @Override + public void hide() + { + super.hide(); + listener.disableShortcut(); + } + private void redrawList() { itemList.clear(); diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java index 3a10359a1d..40663923db 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java @@ -1,17 +1,8 @@ package org.zanata.webtrans.client.presenter; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.mockito.Matchers.isA; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - import java.util.Date; import java.util.List; -import net.customware.gwt.presenter.client.EventBus; - import org.hamcrest.Matchers; import org.mockito.Answers; import org.mockito.ArgumentCaptor; @@ -24,6 +15,7 @@ import org.zanata.webtrans.client.events.CopyDataToEditorEvent; import org.zanata.webtrans.client.events.NotificationEvent; import org.zanata.webtrans.client.events.ReviewCommentEvent; +import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; @@ -38,12 +30,18 @@ import org.zanata.webtrans.shared.rpc.AddReviewCommentResult; import org.zanata.webtrans.shared.rpc.GetTranslationHistoryAction; import org.zanata.webtrans.shared.rpc.GetTranslationHistoryResult; - import com.google.common.collect.Lists; import com.google.gwt.user.cellview.client.ColumnSortEvent; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwt.view.client.SelectionChangeEvent; +import net.customware.gwt.presenter.client.EventBus; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + /** * @author Patrick Huang pahuang@redhat.com */ @@ -72,12 +70,14 @@ public class TranslationHistoryPresenterTest private final TransUnitId transUnitId = new TransUnitId(1L); @Mock(answer = Answers.RETURNS_DEEP_STUBS) private GetTransUnitActionContextHolder contextHolder; + @Mock + private KeyShortcutPresenter keyShortcutPresenter; @BeforeMethod public void beforeMethod() { MockitoAnnotations.initMocks(this); - presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder); + presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder, keyShortcutPresenter); presenter.setCurrentValueHolder(targetContentsPresenter); doNothing().when(dispatcher).execute(actionCaptor.capture(), resultCaptor.capture()); @@ -110,6 +110,8 @@ public void willNotifyErrorAndHideTranslationHistoryOnFailure() verify(eventBus).fireEvent(isA(NotificationEvent.class)); verify(display).hide(); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, true); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, false); } @Test @@ -134,6 +136,8 @@ public void willShowTranslationHistoryOnSuccess() AsyncCallback result = resultCaptor.getValue(); result.onSuccess(createTranslationHistory(latest, historyItem)); verify(display).setData(Lists.newArrayList(latest, historyItem)); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, false); + verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, true); } @Test From 586ad28d2e29f7ada182c3590710a4c3a9c309a2 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Tue, 3 Sep 2013 12:29:02 +1000 Subject: [PATCH 17/39] introduce shortcut context aware dialog box --- .../ForceReviewCommentPresenter.java | 14 ---- .../presenter/KeyShortcutPresenter.java | 22 +++--- .../TranslationHistoryPresenter.java | 19 +---- .../ui/ShortcutContextAwareDialogBox.java | 70 +++++++++++++++++++ .../client/ui/TranslationHistoryDisplay.java | 1 - .../client/ui/TranslationHistoryView.java | 16 ++--- .../client/view/ForceReviewCommentWidget.java | 9 +-- .../ForceReviewCommentPresenterTest.java | 9 +-- .../TranslationHistoryPresenterTest.java | 9 +-- 9 files changed, 94 insertions(+), 75 deletions(-) create mode 100644 zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java index 7b7c0de4e6..d06e7156eb 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java @@ -96,7 +96,6 @@ public void onCommentBeforeSave(CommentBeforeSaveEvent event) { saveEvent = event.getSaveEvent(); display.center(); - enableShortcut(); } @Override @@ -115,19 +114,6 @@ public void onSuccess(AddReviewCommentResult result) display.hide(); } }); - disableShortcut(); - } - - private void enableShortcut() - { - keyShortcutPresenter.setContextActive(ShortcutContext.Edit, false); - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, true); - } - - private void disableShortcut() - { - keyShortcutPresenter.setContextActive(ShortcutContext.Edit, true); - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, false); } @Override diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java index 151e247ffb..dba40dc543 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java @@ -28,9 +28,6 @@ import java.util.Map; import java.util.Set; -import net.customware.gwt.presenter.client.EventBus; -import net.customware.gwt.presenter.client.widget.WidgetPresenter; - import org.zanata.webtrans.client.events.AttentionModeActivationEvent; import org.zanata.webtrans.client.events.KeyShortcutEvent; import org.zanata.webtrans.client.events.KeyShortcutEventHandler; @@ -46,8 +43,8 @@ import org.zanata.webtrans.client.keys.TimerFactory; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.view.KeyShortcutDisplay; - import com.allen_sauer.gwt.log.client.Log; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.gwt.dom.client.NativeEvent; import com.google.gwt.event.dom.client.KeyCodes; @@ -57,6 +54,9 @@ import com.google.gwt.view.client.ListDataProvider; import com.google.inject.Inject; +import net.customware.gwt.presenter.client.EventBus; +import net.customware.gwt.presenter.client.widget.WidgetPresenter; + /** * Detects shortcut key combinations such as Alt+KEY and Shift+Alt+KEY and * broadcasts corresponding {@link KeyShortcutEvent}s. @@ -324,14 +324,9 @@ public void setContextActive(ShortcutContext context, boolean active) { if (context == ShortcutContext.Application) { - // TODO throw exception? Remove this check? Just warn but still - // remove context? - Log.warn("Tried to set global shortcut context inactive. Ignoring."); - } - else - { - ensureActiveContexts().remove(context); + Log.warn("set global shortcut context inactive."); } + ensureActiveContexts().remove(context); } } @@ -471,6 +466,11 @@ public void showShortcuts() display.showPanel(); } + public Set getActiveContexts() + { + return ImmutableSet.copyOf(activeContexts); + } + private class KeyShortcutHandlerRegistration implements HandlerRegistration { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java index 6f3ba1f5b9..ec8b773d3f 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java @@ -44,13 +44,12 @@ public class TranslationHistoryPresenter extends WidgetPresenter otherEntries, List reviewComments) @@ -158,19 +154,6 @@ public void compareClicked(TransHistoryItem item) } } - private void enableShortcut() - { - keyShortcutPresenter.setContextActive(ShortcutContext.Edit, false); - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, true); - } - - @Override - public void disableShortcut() - { - keyShortcutPresenter.setContextActive(ShortcutContext.Edit, true); - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, false); - } - @Override public boolean isItemInComparison(TransHistoryItem item) { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java new file mode 100644 index 0000000000..da4d6862e0 --- /dev/null +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java @@ -0,0 +1,70 @@ +package org.zanata.webtrans.client.ui; + +import java.util.Collections; +import java.util.Set; + +import org.zanata.webtrans.client.keys.ShortcutContext; +import org.zanata.webtrans.client.presenter.KeyShortcutPresenter; +import com.google.gwt.user.client.ui.DialogBox; + +/** + * @author Patrick Huang pahuang@redhat.com + */ +public class ShortcutContextAwareDialogBox extends DialogBox +{ + private final KeyShortcutPresenter keyShortcutPresenter; + private Set activeContext = Collections.emptySet(); + + public ShortcutContextAwareDialogBox(KeyShortcutPresenter keyShortcutPresenter) + { + super(); + this.keyShortcutPresenter = keyShortcutPresenter; + } + + public ShortcutContextAwareDialogBox(boolean autoHide, KeyShortcutPresenter keyShortcutPresenter) + { + super(autoHide); + this.keyShortcutPresenter = keyShortcutPresenter; + } + + public ShortcutContextAwareDialogBox(Caption captionWidget, KeyShortcutPresenter keyShortcutPresenter) + { + super(captionWidget); + this.keyShortcutPresenter = keyShortcutPresenter; + } + + public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, KeyShortcutPresenter keyShortcutPresenter) + { + super(autoHide, modal); + this.keyShortcutPresenter = keyShortcutPresenter; + } + + public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, Caption captionWidget, KeyShortcutPresenter keyShortcutPresenter) + { + super(autoHide, modal, captionWidget); + this.keyShortcutPresenter = keyShortcutPresenter; + } + + @Override + public void hide() + { + super.hide(); + keyShortcutPresenter.setContextActive(ShortcutContext.Popup, false); + for (ShortcutContext shortcutContext : activeContext) + { + keyShortcutPresenter.setContextActive(shortcutContext, true); + } + } + + @Override + public void show() + { + super.show(); + activeContext = keyShortcutPresenter.getActiveContexts(); + for (ShortcutContext shortcutContext : activeContext) + { + keyShortcutPresenter.setContextActive(shortcutContext, false); + } + keyShortcutPresenter.setContextActive(ShortcutContext.Popup, true); + } +} diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java index e5b7c56b82..8a233e2df5 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java @@ -43,6 +43,5 @@ interface Listener extends ForceReviewCommentDisplay.Listener boolean isItemInComparison(TransHistoryItem item); - void disableShortcut(); } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java index 2c710079b9..8eacdfb254 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java @@ -2,6 +2,7 @@ import java.util.List; +import org.zanata.webtrans.client.presenter.KeyShortcutPresenter; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.shared.model.ComparableByDate; import org.zanata.webtrans.shared.model.ReviewComment; @@ -11,16 +12,14 @@ import com.google.gwt.resources.client.CssResource; import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiField; -import com.google.gwt.user.client.ui.DialogBox; import com.google.gwt.user.client.ui.HTMLPanel; import com.google.gwt.user.client.ui.TabLayoutPanel; import com.google.inject.Inject; import com.google.inject.Singleton; @Singleton -public class TranslationHistoryView extends DialogBox implements TranslationHistoryDisplay +public class TranslationHistoryView extends ShortcutContextAwareDialogBox implements TranslationHistoryDisplay { - private static final int COMPARISON_TAB_INDEX = 1; private static TranslationHistoryViewUiBinder uiBinder = GWT.create(TranslationHistoryViewUiBinder.class); private final ContentStateRenderer stateRenderer; @UiField @@ -45,9 +44,9 @@ public class TranslationHistoryView extends DialogBox implements TranslationHist private List items = Lists.newArrayList(); @Inject - public TranslationHistoryView(ContentStateRenderer stateRenderer) + public TranslationHistoryView(ContentStateRenderer stateRenderer, KeyShortcutPresenter keyShortcutPresenter) { - super(true, true); + super(true, true, keyShortcutPresenter); this.stateRenderer = stateRenderer; closeButton = new DialogBoxCloseButton(this); HTMLPanel container = uiBinder.createAndBindUi(this); @@ -66,13 +65,6 @@ public void setData(List items) redrawList(); } - @Override - public void hide() - { - super.hide(); - listener.disableShortcut(); - } - private void redrawList() { itemList.clear(); diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java index fc4057a622..97bda6f9b0 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java @@ -21,10 +21,11 @@ package org.zanata.webtrans.client.view; +import org.zanata.webtrans.client.presenter.KeyShortcutPresenter; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.ui.DialogBoxCloseButton; import org.zanata.webtrans.client.ui.ReviewCommentInputWidget; -import com.google.gwt.user.client.ui.DialogBox; +import org.zanata.webtrans.client.ui.ShortcutContextAwareDialogBox; import com.google.gwt.user.client.ui.FlowPanel; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -33,14 +34,14 @@ * @author Patrick Huang pahuang@redhat.com */ @Singleton -public class ForceReviewCommentWidget extends DialogBox implements ForceReviewCommentDisplay +public class ForceReviewCommentWidget extends ShortcutContextAwareDialogBox implements ForceReviewCommentDisplay { private final ReviewCommentInputWidget inputWidget; @Inject - public ForceReviewCommentWidget(WebTransMessages messages) + public ForceReviewCommentWidget(WebTransMessages messages, KeyShortcutPresenter keyShortcutPresenter) { - super(false, true); + super(false, true, keyShortcutPresenter); setGlassEnabled(true); setText(messages.rejectCommentTitle()); diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java index c8e01310f0..d704ef4806 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java @@ -26,7 +26,6 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; -import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -43,12 +42,12 @@ import org.zanata.webtrans.shared.model.ReviewComment; import org.zanata.webtrans.shared.rpc.AddReviewCommentAction; import org.zanata.webtrans.shared.rpc.AddReviewCommentResult; - import com.google.gwt.user.client.rpc.AsyncCallback; import net.customware.gwt.presenter.client.EventBus; import static org.hamcrest.MatcherAssert.assertThat; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; /** * @author Patrick Huang pahuang@redhat.com @@ -97,8 +96,6 @@ public void testOnCommentBeforeSave() throws Exception presenter.onCommentBeforeSave(commentBeforeSaveEvent); verify(display).center(); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, false); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, true); } @Test @@ -124,7 +121,5 @@ public void testAddComment() throws Exception verify(eventBus).fireEvent(saveEvent); verify(eventBus).fireEvent(NavTransUnitEvent.NEXT_ENTRY_EVENT); verify(display).hide(); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, true); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, false); } } diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java index 40663923db..9871d70fea 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java @@ -15,7 +15,6 @@ import org.zanata.webtrans.client.events.CopyDataToEditorEvent; import org.zanata.webtrans.client.events.NotificationEvent; import org.zanata.webtrans.client.events.ReviewCommentEvent; -import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; @@ -70,14 +69,12 @@ public class TranslationHistoryPresenterTest private final TransUnitId transUnitId = new TransUnitId(1L); @Mock(answer = Answers.RETURNS_DEEP_STUBS) private GetTransUnitActionContextHolder contextHolder; - @Mock - private KeyShortcutPresenter keyShortcutPresenter; @BeforeMethod public void beforeMethod() { MockitoAnnotations.initMocks(this); - presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder, keyShortcutPresenter); + presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder); presenter.setCurrentValueHolder(targetContentsPresenter); doNothing().when(dispatcher).execute(actionCaptor.capture(), resultCaptor.capture()); @@ -110,8 +107,6 @@ public void willNotifyErrorAndHideTranslationHistoryOnFailure() verify(eventBus).fireEvent(isA(NotificationEvent.class)); verify(display).hide(); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, true); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, false); } @Test @@ -136,8 +131,6 @@ public void willShowTranslationHistoryOnSuccess() AsyncCallback result = resultCaptor.getValue(); result.onSuccess(createTranslationHistory(latest, historyItem)); verify(display).setData(Lists.newArrayList(latest, historyItem)); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Edit, false); - verify(keyShortcutPresenter).setContextActive(ShortcutContext.Popup, true); } @Test From daa499f07d39165ed75ce337a5d522231fab00e0 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 12:20:55 +1000 Subject: [PATCH 18/39] clean up import --- .../main/java/org/zanata/page/groups/VersionGroupPage.java | 3 +-- .../test/java/org/zanata/concordion/GeneratedIndexSource.java | 4 ---- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java b/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java index 7f5f2b90c4..d354952efe 100644 --- a/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java +++ b/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java @@ -15,7 +15,6 @@ import org.zanata.util.TableRow; import org.zanata.util.WebElementUtil; -import javax.annotation.Nullable; import java.util.List; @@ -113,7 +112,7 @@ public VersionGroupPage closeSearchResult(final int expectedRow) return refreshPageUntil(this, new Predicate() { @Override - public boolean apply(@Nullable WebDriver input) + public boolean apply(WebDriver input) { List tableRows = WebElementUtil.getTableRows(input, versionsInGroupTableBy); int size = tableRows.size(); diff --git a/functional-test/src/test/java/org/zanata/concordion/GeneratedIndexSource.java b/functional-test/src/test/java/org/zanata/concordion/GeneratedIndexSource.java index 0df1aa3e12..08596eaf97 100644 --- a/functional-test/src/test/java/org/zanata/concordion/GeneratedIndexSource.java +++ b/functional-test/src/test/java/org/zanata/concordion/GeneratedIndexSource.java @@ -7,11 +7,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; -import java.util.ArrayList; import java.util.List; -import javax.annotation.Nullable; - import org.concordion.api.Resource; import org.concordion.internal.ClassPathSource; import org.junit.runners.Suite; @@ -20,7 +17,6 @@ import com.google.common.base.Joiner; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import lombok.extern.slf4j.Slf4j; From 987968966853b50369f10d7376737797bb8b67b3 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 12:22:12 +1000 Subject: [PATCH 19/39] rhbz996446 - translation history popup shortcut context --- .../webtrans/client/keys/ShortcutContext.java | 9 ++++-- .../ForceReviewCommentPresenter.java | 9 +++--- .../presenter/KeyShortcutPresenter.java | 2 +- .../TranslationHistoryPresenter.java | 27 +++++++++++++++- .../client/resources/WebTransMessages.java | 2 +- .../client/ui/ReviewCommentInputWidget.ui.xml | 2 +- .../ui/ShortcutContextAwareDialogBox.java | 32 +++---------------- .../client/ui/TranslationHistoryDisplay.java | 2 ++ .../client/ui/TranslationHistoryView.java | 9 +++++- .../client/view/ForceReviewCommentWidget.java | 3 +- .../ForceReviewCommentPresenterTest.java | 7 ++-- .../TranslationHistoryPresenterTest.java | 26 ++++++++++++++- 12 files changed, 84 insertions(+), 46 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java b/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java index 6c7b00809c..d2a0e8ec49 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/keys/ShortcutContext.java @@ -69,8 +69,13 @@ public enum ShortcutContext Chat, /** - * Used by all popup + * Used by {@link org.zanata.webtrans.client.presenter.TranslationHistoryPresenter} */ - Popup + TransHistoryPopup, + + /** + * Used by {@link org.zanata.webtrans.client.presenter.ForceReviewCommentPresenter} + */ + RejectConfirmationPopup } \ No newline at end of file diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java index d06e7156eb..42d434fe10 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenter.java @@ -59,8 +59,7 @@ public class ForceReviewCommentPresenter extends WidgetPresenter getActiveContexts() { - return ImmutableSet.copyOf(activeContexts); + return ImmutableSet.copyOf(ensureActiveContexts()); } private class KeyShortcutHandlerRegistration implements HandlerRegistration diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java index ec8b773d3f..903b4f1b57 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenter.java @@ -5,9 +5,13 @@ import java.util.List; import org.zanata.webtrans.client.events.CopyDataToEditorEvent; +import org.zanata.webtrans.client.events.KeyShortcutEvent; +import org.zanata.webtrans.client.events.KeyShortcutEventHandler; import org.zanata.webtrans.client.events.NotificationEvent; import org.zanata.webtrans.client.events.ReviewCommentEvent; import org.zanata.webtrans.client.events.ReviewCommentEventHandler; +import org.zanata.webtrans.client.keys.KeyShortcut; +import org.zanata.webtrans.client.keys.Keys; import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.AbstractAsyncCallback; @@ -25,6 +29,7 @@ import com.allen_sauer.gwt.log.client.Log; import com.google.common.base.Objects; import com.google.common.collect.Lists; +import com.google.gwt.event.dom.client.KeyCodes; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.inject.Inject; import com.google.inject.Singleton; @@ -44,12 +49,13 @@ public class TranslationHistoryPresenter extends WidgetPresenter
- +
diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java index da4d6862e0..1294f84f8c 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java @@ -12,36 +12,14 @@ */ public class ShortcutContextAwareDialogBox extends DialogBox { + private final ShortcutContext shortcutContext; private final KeyShortcutPresenter keyShortcutPresenter; private Set activeContext = Collections.emptySet(); - public ShortcutContextAwareDialogBox(KeyShortcutPresenter keyShortcutPresenter) - { - super(); - this.keyShortcutPresenter = keyShortcutPresenter; - } - - public ShortcutContextAwareDialogBox(boolean autoHide, KeyShortcutPresenter keyShortcutPresenter) - { - super(autoHide); - this.keyShortcutPresenter = keyShortcutPresenter; - } - - public ShortcutContextAwareDialogBox(Caption captionWidget, KeyShortcutPresenter keyShortcutPresenter) - { - super(captionWidget); - this.keyShortcutPresenter = keyShortcutPresenter; - } - - public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, KeyShortcutPresenter keyShortcutPresenter) + public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, ShortcutContext shortcutContext, KeyShortcutPresenter keyShortcutPresenter) { super(autoHide, modal); - this.keyShortcutPresenter = keyShortcutPresenter; - } - - public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, Caption captionWidget, KeyShortcutPresenter keyShortcutPresenter) - { - super(autoHide, modal, captionWidget); + this.shortcutContext = shortcutContext; this.keyShortcutPresenter = keyShortcutPresenter; } @@ -49,11 +27,11 @@ public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, Caption ca public void hide() { super.hide(); - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, false); for (ShortcutContext shortcutContext : activeContext) { keyShortcutPresenter.setContextActive(shortcutContext, true); } + keyShortcutPresenter.setContextActive(shortcutContext, false); } @Override @@ -65,6 +43,6 @@ public void show() { keyShortcutPresenter.setContextActive(shortcutContext, false); } - keyShortcutPresenter.setContextActive(ShortcutContext.Popup, true); + keyShortcutPresenter.setContextActive(shortcutContext, true); } } diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java index 8a233e2df5..62cd30d604 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryDisplay.java @@ -34,6 +34,8 @@ public interface TranslationHistoryDisplay extends WidgetDisplay void clearInput(); + String getComment(); + interface Listener extends ForceReviewCommentDisplay.Listener { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java index 8eacdfb254..3e11891e30 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/TranslationHistoryView.java @@ -2,6 +2,7 @@ import java.util.List; +import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.presenter.KeyShortcutPresenter; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.shared.model.ComparableByDate; @@ -46,7 +47,7 @@ public class TranslationHistoryView extends ShortcutContextAwareDialogBox implem @Inject public TranslationHistoryView(ContentStateRenderer stateRenderer, KeyShortcutPresenter keyShortcutPresenter) { - super(true, true, keyShortcutPresenter); + super(true, true, ShortcutContext.TransHistoryPopup, keyShortcutPresenter); this.stateRenderer = stateRenderer; closeButton = new DialogBoxCloseButton(this); HTMLPanel container = uiBinder.createAndBindUi(this); @@ -94,6 +95,12 @@ public void clearInput() commentInput.setText(""); } + @Override + public String getComment() + { + return commentInput.getText(); + } + @Override public void showDiff(TransHistoryItem one, TransHistoryItem two, String description) { diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java index 97bda6f9b0..15452dcaaf 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/view/ForceReviewCommentWidget.java @@ -21,6 +21,7 @@ package org.zanata.webtrans.client.view; +import org.zanata.webtrans.client.keys.ShortcutContext; import org.zanata.webtrans.client.presenter.KeyShortcutPresenter; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.ui.DialogBoxCloseButton; @@ -41,7 +42,7 @@ public class ForceReviewCommentWidget extends ShortcutContextAwareDialogBox impl @Inject public ForceReviewCommentWidget(WebTransMessages messages, KeyShortcutPresenter keyShortcutPresenter) { - super(false, true, keyShortcutPresenter); + super(false, true, ShortcutContext.RejectConfirmationPopup, keyShortcutPresenter); setGlassEnabled(true); setText(messages.rejectCommentTitle()); diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java index d704ef4806..37e37ec95f 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/ForceReviewCommentPresenterTest.java @@ -34,7 +34,6 @@ import org.zanata.webtrans.client.events.TransUnitSaveEvent; import org.zanata.webtrans.client.keys.KeyShortcut; import org.zanata.webtrans.client.keys.ShortcutContext; -import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; import org.zanata.webtrans.client.view.ForceReviewCommentDisplay; @@ -70,8 +69,6 @@ public class ForceReviewCommentPresenterTest private TransUnitSaveEvent saveEvent; @Mock private KeyShortcutPresenter keyShortcutPresenter; - @Mock - private WebTransMessages messages; @Captor private ArgumentCaptor shortcutCapture; @@ -80,14 +77,14 @@ public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - presenter = new ForceReviewCommentPresenter(display, eventBus, dispatcher, contextHolder, keyShortcutPresenter, messages); + presenter = new ForceReviewCommentPresenter(display, eventBus, dispatcher, contextHolder, keyShortcutPresenter); verify(display).setListener(presenter); verify(eventBus).addHandler(CommentBeforeSaveEvent.TYPE, presenter); verify(keyShortcutPresenter).register(shortcutCapture.capture()); KeyShortcut keyShortcut = shortcutCapture.getValue(); - assertThat(keyShortcut.getContext(), Matchers.equalTo(ShortcutContext.Popup)); + assertThat(keyShortcut.getContext(), Matchers.equalTo(ShortcutContext.RejectConfirmationPopup)); } @Test diff --git a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java index 9871d70fea..6fb661c89e 100644 --- a/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java +++ b/zanata-war/src/test/java/org/zanata/webtrans/client/presenter/TranslationHistoryPresenterTest.java @@ -8,13 +8,17 @@ import org.mockito.ArgumentCaptor; import org.mockito.Captor; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import org.zanata.common.ContentState; import org.zanata.webtrans.client.events.CopyDataToEditorEvent; +import org.zanata.webtrans.client.events.KeyShortcutEvent; import org.zanata.webtrans.client.events.NotificationEvent; import org.zanata.webtrans.client.events.ReviewCommentEvent; +import org.zanata.webtrans.client.keys.KeyShortcut; +import org.zanata.webtrans.client.keys.Keys; import org.zanata.webtrans.client.resources.WebTransMessages; import org.zanata.webtrans.client.rpc.CachingDispatchAsync; import org.zanata.webtrans.client.service.GetTransUnitActionContextHolder; @@ -30,6 +34,7 @@ import org.zanata.webtrans.shared.rpc.GetTranslationHistoryAction; import org.zanata.webtrans.shared.rpc.GetTranslationHistoryResult; import com.google.common.collect.Lists; +import com.google.gwt.event.dom.client.KeyCodes; import com.google.gwt.user.cellview.client.ColumnSortEvent; import com.google.gwt.user.client.rpc.AsyncCallback; import com.google.gwt.view.client.SelectionChangeEvent; @@ -38,6 +43,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -69,16 +75,22 @@ public class TranslationHistoryPresenterTest private final TransUnitId transUnitId = new TransUnitId(1L); @Mock(answer = Answers.RETURNS_DEEP_STUBS) private GetTransUnitActionContextHolder contextHolder; + @Mock + private KeyShortcutPresenter keyShortcutPresenter; + @Captor + private ArgumentCaptor keyShortcutCapture; @BeforeMethod public void beforeMethod() { MockitoAnnotations.initMocks(this); - presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder); + presenter = new TranslationHistoryPresenter(display, eventBus, dispatcher, messages, contextHolder, keyShortcutPresenter); presenter.setCurrentValueHolder(targetContentsPresenter); doNothing().when(dispatcher).execute(actionCaptor.capture(), resultCaptor.capture()); verify(eventBus).addHandler(ReviewCommentEvent.TYPE, presenter); + + verify(keyShortcutPresenter).register(keyShortcutCapture.capture()); } private static TransHistoryItem historyItem(String versionNum) @@ -239,4 +251,16 @@ public void onCompareClickedWhichMakesTwoItems() } + @Test + public void testKeyShortcutForAddComment() + { + when(display.getComment()).thenReturn("blah"); + KeyShortcut keyShortcut = keyShortcutCapture.getValue(); + + assertThat(keyShortcut.getAllKeys().iterator().next(), Matchers.equalTo(new Keys(Keys.CTRL_KEY, KeyCodes.KEY_ENTER))); + keyShortcut.getHandler().onKeyShortcut(mock(KeyShortcutEvent.class)); + + verify(display).getComment(); + verify(dispatcher).execute(Mockito.isA(AddReviewCommentAction.class), Mockito.isA(AsyncCallback.class)); + } } From 6ae2fa8270fb95459f03469628389d562ca943bb Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 12:53:33 +1000 Subject: [PATCH 20/39] fix dependency problem --- functional-test/pom.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/functional-test/pom.xml b/functional-test/pom.xml index 08076fca06..ce1c587fb4 100644 --- a/functional-test/pom.xml +++ b/functional-test/pom.xml @@ -113,6 +113,12 @@ com.google.guava guava + + + com.google.code.findbugs + jsr305 + + From 03f36a9cc4f5788142226785c9c4e9322733b919 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 13:13:14 +1000 Subject: [PATCH 21/39] bring back the dependency use --- functional-test/pom.xml | 6 ------ .../main/java/org/zanata/page/groups/VersionGroupPage.java | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/functional-test/pom.xml b/functional-test/pom.xml index ce1c587fb4..08076fca06 100644 --- a/functional-test/pom.xml +++ b/functional-test/pom.xml @@ -113,12 +113,6 @@ com.google.guava guava - - - com.google.code.findbugs - jsr305 - - diff --git a/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java b/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java index d354952efe..e3cbaf55cc 100644 --- a/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java +++ b/functional-test/src/main/java/org/zanata/page/groups/VersionGroupPage.java @@ -112,7 +112,7 @@ public VersionGroupPage closeSearchResult(final int expectedRow) return refreshPageUntil(this, new Predicate() { @Override - public boolean apply(WebDriver input) + public boolean apply(@javax.annotation.Nullable WebDriver input) { List tableRows = WebElementUtil.getTableRows(input, versionsInGroupTableBy); int size = tableRows.size(); From 4ed915d53813ff0ca248c7adec6a25559e68c083 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 10:40:13 +1000 Subject: [PATCH 22/39] Add .gitattributes to ensure correct line endings Ref: https://help.github.com/articles/dealing-with-line-endings --- .gitattributes | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000000..39aa72a521 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,35 @@ +# See https://help.github.com/articles/dealing-with-line-endings + +# Handle line endings automatically for files detected as text +# and leave all files detected as binary untouched. +* text=auto + +# Explicitly declare text files we want to always be normalized and converted +# to native line endings on checkout. +*.c text +*.css text +*.csv text +*.groovy text +*.h text +*.html text +*.ini text +*.java text +*.js text +*.launch text +*.po text +*.pot text +*.properties text +*.sh text +*.sql text +*.txt text +*.xhtml text +*.xml text + +# Declare files that will always have CRLF line endings on checkout. +*.sln text eol=crlf + +# Denote all files that are truly binary and should not be modified. +*.gif binary +*.jar binary +*.jpg binary +*.png binary From 753c3d1e5f24c3d3866567b1f9f9a5eadb26ad99 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 15:54:17 +1000 Subject: [PATCH 23/39] Preserve whitespace in enforcer messages --- zanata-war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index 1a90711fb9..29113fa9f0 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -127,7 +127,7 @@ arquillian.jboss.home - You must set the arquillian.jboss.home property to run integration tests + You must set the arquillian.jboss.home property to run integration tests From a9a6a2e78b26d4909824518429635a626b8bb36c Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Thu, 5 Sep 2013 16:08:48 +1000 Subject: [PATCH 24/39] move modal context logic to KeyShortcutPresenter --- .../presenter/KeyShortcutPresenter.java | 21 +++++++++++++++++-- .../ui/ShortcutContextAwareDialogBox.java | 21 ++++++------------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java index 8321b4382e..55b624fb04 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/presenter/KeyShortcutPresenter.java @@ -87,6 +87,8 @@ public class KeyShortcutPresenter extends WidgetPresenter // TODO unregister when user changes attention shortcut private HandlerRegistration attentionShortcutHandle; + private transient ShortcutContext modalContext; + private transient Set copyOfCurrentContexts =Collections.emptySet(); @Inject public KeyShortcutPresenter(KeyShortcutDisplay display, @@ -466,9 +468,24 @@ public void showShortcuts() display.showPanel(); } - public Set getActiveContexts() + public void deactivateModalContext() { - return ImmutableSet.copyOf(ensureActiveContexts()); + setContextActive(modalContext, false); + for (ShortcutContext context : copyOfCurrentContexts) + { + setContextActive(context, true); + } + } + + public void activateModalContext(ShortcutContext modalContext) + { + this.modalContext = modalContext; + copyOfCurrentContexts = ImmutableSet.copyOf(ensureActiveContexts()); + for (ShortcutContext context : copyOfCurrentContexts) + { + setContextActive(context, false); + } + setContextActive(modalContext, true); } private class KeyShortcutHandlerRegistration implements HandlerRegistration diff --git a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java index 1294f84f8c..b42385cd26 100644 --- a/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java +++ b/zanata-war/src/main/java/org/zanata/webtrans/client/ui/ShortcutContextAwareDialogBox.java @@ -12,14 +12,13 @@ */ public class ShortcutContextAwareDialogBox extends DialogBox { - private final ShortcutContext shortcutContext; + private final ShortcutContext modalContext; private final KeyShortcutPresenter keyShortcutPresenter; - private Set activeContext = Collections.emptySet(); - public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, ShortcutContext shortcutContext, KeyShortcutPresenter keyShortcutPresenter) + public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, ShortcutContext modalContext, KeyShortcutPresenter keyShortcutPresenter) { super(autoHide, modal); - this.shortcutContext = shortcutContext; + this.modalContext = modalContext; this.keyShortcutPresenter = keyShortcutPresenter; } @@ -27,22 +26,14 @@ public ShortcutContextAwareDialogBox(boolean autoHide, boolean modal, ShortcutCo public void hide() { super.hide(); - for (ShortcutContext shortcutContext : activeContext) - { - keyShortcutPresenter.setContextActive(shortcutContext, true); - } - keyShortcutPresenter.setContextActive(shortcutContext, false); + keyShortcutPresenter.deactivateModalContext(); + } @Override public void show() { super.show(); - activeContext = keyShortcutPresenter.getActiveContexts(); - for (ShortcutContext shortcutContext : activeContext) - { - keyShortcutPresenter.setContextActive(shortcutContext, false); - } - keyShortcutPresenter.setContextActive(shortcutContext, true); + keyShortcutPresenter.activateModalContext(modalContext); } } From 60c9dc5b2771b297e4a7b936e6b0fb02095be48e Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 22:49:19 +1000 Subject: [PATCH 25/39] Fix XML files to pass validation --- zanata-war/src/main/webapp-jboss/WEB-INF/jboss-web.xml | 9 +++++---- zanata-war/src/main/webapp/WEB-INF/faces-config.xml | 7 +++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/zanata-war/src/main/webapp-jboss/WEB-INF/jboss-web.xml b/zanata-war/src/main/webapp-jboss/WEB-INF/jboss-web.xml index 69b2cac921..a0b06cee03 100644 --- a/zanata-war/src/main/webapp-jboss/WEB-INF/jboss-web.xml +++ b/zanata-war/src/main/webapp-jboss/WEB-INF/jboss-web.xml @@ -1,13 +1,14 @@ - - + + - - org.zanata.security.FedoraOpenIdPhaseListener - - + + org.zanata.security.FedoraOpenIdPhaseListener + From 6e011850dddfc96d9b695dfa0398acce28deca7a Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Thu, 5 Sep 2013 23:41:04 +1000 Subject: [PATCH 26/39] Enable Groovy for src; switch from GMaven to groovy-eclipse-compiler --- zanata-war/pom.xml | 79 ++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index 29113fa9f0..a188f6399e 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -35,9 +35,10 @@ ${org.projectlombok:lombok:jar} - - 2.0 + 2.0.7 + 2.0.7-03 + 2.8.0-01 1 @@ -53,6 +54,41 @@ + + + maven-compiler-plugin + + 3.0 + + groovy-eclipse-compiler + + + + + lombok.core.Agent + + true + + + + org.codehaus.groovy + groovy-eclipse-compiler + ${groovy.eclipse.compiler.version} + + + + org.codehaus.groovy + groovy-eclipse-batch + ${groovy.eclipse.batch.version} + + + org.projectlombok + lombok + ${lombok.version} + + + + maven-dependency-plugin @@ -535,45 +571,6 @@ - - org.codehaus.gmaven - gmaven-plugin - 1.4 - - ${gmavenProviderSelection} - UTF-8 - - - - - - - - generateTestStubs - testCompile - - - - - - org.codehaus.gmaven.runtime - gmaven-runtime-2.0 - 1.4 - - - org.codehaus.groovy - groovy-all - - - - - org.codehaus.groovy - groovy-all - ${groovyVersion} - - - - From ea1270c5aa2fd850c12e62f5d2ec26b25712513d Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Fri, 6 Sep 2013 00:01:52 +1000 Subject: [PATCH 27/39] Update to Groovy 2.1.5 --- zanata-war/pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index a188f6399e..ad9a365089 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -36,8 +36,8 @@ ${org.projectlombok:lombok:jar} - 2.0.7 - 2.0.7-03 + 2.1.5 + 2.1.5-03 2.8.0-01 1 From cf9c5fd57fdaeb79f2551b76687a4db35cbc0ba5 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 6 Sep 2013 11:47:38 +1000 Subject: [PATCH 28/39] rhbz1002774 - change HTextFlow.setContents logic to ensure correctness --- .../main/java/org/zanata/model/HTextFlow.java | 24 +++++---- .../java/org/zanata/model/HTextFlowTest.java | 53 +++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) create mode 100644 zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java diff --git a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java index e7b1332ac4..4fca91e64e 100644 --- a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java +++ b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java @@ -282,13 +282,22 @@ public List getContents() return contents; } - public void setContents(List contents) + public void setContents(List newContents) { - if(!Objects.equal(contents, this.getContents())) + if (!newContents.equals(this.getContents())) { - for( int i=0; i 0) + { + for (int i = newContentSize; i < newContentSize + difference; i++) + { + setContent(i, null); + } } updateContentHash(); updateWordCount(); @@ -510,12 +519,7 @@ private void preUpdate() if (!isPlural()) { // if plural form has changed, we need to clear out obsolete contents - List newContents = Lists.newArrayList(content0); - for (int i = 1; i < MAX_PLURALS; i++) - { - newContents.add(null); - } - setContents(newContents); + setContents(content0); } } } diff --git a/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java b/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java new file mode 100644 index 0000000000..7c2fefafb2 --- /dev/null +++ b/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java @@ -0,0 +1,53 @@ +/* + * Copyright 2013, Red Hat, Inc. and individual contributors as indicated by the + * @author tags. See the copyright.txt file in the distribution for a full + * listing of individual contributors. + * + * This is free software; you can redistribute it and/or modify it under the + * terms of the GNU Lesser General Public License as published by the Free + * Software Foundation; either version 2.1 of the License, or (at your option) + * any later version. + * + * This software is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS + * FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more + * details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this software; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA, or see the FSF + * site: http://www.fsf.org. + */ + +package org.zanata.model; + +import org.hamcrest.Matchers; +import org.testng.annotations.Test; + +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * @author Patrick Huang pahuang@redhat.com + */ +@Test(groups = "unit-tests") +public class HTextFlowTest +{ + @Test + public void testSetContents() throws Exception + { + HTextFlow textFlow = new HTextFlow(); + + textFlow.setContents("a", "b"); + + assertThat(textFlow.getContents(), Matchers.contains("a", "b")); + + textFlow.setContents("a"); + + assertThat(textFlow.getContents(), Matchers.contains("a")); + assertThat(textFlow.getContent1(), Matchers.nullValue()); + + textFlow.setContents("a", "b"); + assertThat(textFlow.getContents(), Matchers.contains("a", "b")); + + } +} From 9b975739daeb0be70fa51ab3808f564f8057419f Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Fri, 6 Sep 2013 11:43:42 +1000 Subject: [PATCH 29/39] Downgrade groovy-eclipse-compiler to avoid GRECLIPSE-1661 --- zanata-war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index ad9a365089..cb05e9709f 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -38,7 +38,7 @@ 2.1.5 2.1.5-03 - 2.8.0-01 + 2.7.0-01 1 From 91c2fd5a187a5ff2bca14178daabb542eda5a05d Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Fri, 6 Sep 2013 12:20:36 +1000 Subject: [PATCH 30/39] Simplify code for setContents --- .../main/java/org/zanata/model/HTextFlow.java | 14 +++----------- .../java/org/zanata/model/HTextFlowTest.java | 18 +++++++++++------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java index 4fca91e64e..a7dd3a0692 100644 --- a/zanata-model/src/main/java/org/zanata/model/HTextFlow.java +++ b/zanata-model/src/main/java/org/zanata/model/HTextFlow.java @@ -286,18 +286,10 @@ public void setContents(List newContents) { if (!newContents.equals(this.getContents())) { - int newContentSize = newContents.size(); - for (int i = 0; i < newContentSize; i++) + for (int i = 0; i < MAX_PLURALS; i++) { - this.setContent(i, newContents.get(i)); - } - int difference = getContents().size() - newContentSize; - if (difference > 0) - { - for (int i = newContentSize; i < newContentSize + difference; i++) - { - setContent(i, null); - } + String value = i < newContents.size() ? newContents.get(i) : null; + this.setContent(i, value); } updateContentHash(); updateWordCount(); diff --git a/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java b/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java index 7c2fefafb2..53656b1cd2 100644 --- a/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java +++ b/zanata-model/src/test/java/org/zanata/model/HTextFlowTest.java @@ -21,11 +21,12 @@ package org.zanata.model; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; + import org.hamcrest.Matchers; import org.testng.annotations.Test; -import static org.hamcrest.MatcherAssert.assertThat; - /** * @author Patrick Huang pahuang@redhat.com */ @@ -38,16 +39,19 @@ public void testSetContents() throws Exception HTextFlow textFlow = new HTextFlow(); textFlow.setContents("a", "b"); - - assertThat(textFlow.getContents(), Matchers.contains("a", "b")); + assertThat(textFlow.getContents(), contains("a", "b")); textFlow.setContents("a"); - - assertThat(textFlow.getContents(), Matchers.contains("a")); + assertThat(textFlow.getContents(), contains("a")); + // check that content1 is nulled out (after having been non-null earlier) assertThat(textFlow.getContent1(), Matchers.nullValue()); + // set original value textFlow.setContents("a", "b"); - assertThat(textFlow.getContents(), Matchers.contains("a", "b")); + assertThat(textFlow.getContents(), contains("a", "b")); + // set same value + textFlow.setContents("a", "b"); + assertThat(textFlow.getContents(), contains("a", "b")); } } From 17bf8e97963a526f89a99c9d0635b4bc99161bec Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Fri, 6 Sep 2013 13:45:16 +1000 Subject: [PATCH 31/39] change groovy scope so IntelliJ can compile --- zanata-war/pom.xml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/zanata-war/pom.xml b/zanata-war/pom.xml index cb05e9709f..8e217cba88 100644 --- a/zanata-war/pom.xml +++ b/zanata-war/pom.xml @@ -36,7 +36,7 @@ ${org.projectlombok:lombok:jar} - 2.1.5 + 2.1.5 2.1.5-03 2.7.0-01 @@ -1715,12 +1715,10 @@ - org.codehaus.groovy groovy-all - ${groovyVersion} - test + ${groovy.version} From 8bc0cedfd9205bfe1fc643528df505c12b2e3a35 Mon Sep 17 00:00:00 2001 From: Sean Flanigan Date: Fri, 6 Sep 2013 16:44:07 +1000 Subject: [PATCH 32/39] Update to Seam 2.3.1 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2a95190843..3fc43ba090 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ 4.8 ${project.build.sourceDirectory}/org/zanata 3.5.0 - 2.3.0.Final + 2.3.1.Final 1.2.1 0.22 From 2f1f7c407db89f1df53d8d565832c5f70cce23d9 Mon Sep 17 00:00:00 2001 From: Alex Eng Date: Mon, 9 Sep 2013 13:50:28 +1000 Subject: [PATCH 33/39] update zanata common version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 2a95190843..5189cb1076 100644 --- a/pom.xml +++ b/pom.xml @@ -36,7 +36,7 @@ 3.0.1 3.0.1 - 3.0.2-SNAPSHOT + 3.0.2 4.3.2.Final From ba349936fa7f4669411e53c16102954c7f57cfcb Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 9 Sep 2013 13:39:48 +1000 Subject: [PATCH 34/39] rhbz1002340 - verify username availability in first openid log in --- .../src/test/resources/conf/standalone.xml | 6 ++++ .../java/org/zanata/action/ProfileAction.java | 30 ++++++++++++------- zanata-war/src/main/webapp/profile/edit.xhtml | 3 +- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/functional-test/src/test/resources/conf/standalone.xml b/functional-test/src/test/resources/conf/standalone.xml index 3f15311ac6..2d91872b6b 100644 --- a/functional-test/src/test/resources/conf/standalone.xml +++ b/functional-test/src/test/resources/conf/standalone.xml @@ -218,6 +218,7 @@ + @@ -262,6 +263,11 @@ + + + + + diff --git a/zanata-war/src/main/java/org/zanata/action/ProfileAction.java b/zanata-war/src/main/java/org/zanata/action/ProfileAction.java index 27e6e4513b..6c4e23b5c8 100644 --- a/zanata-war/src/main/java/org/zanata/action/ProfileAction.java +++ b/zanata-war/src/main/java/org/zanata/action/ProfileAction.java @@ -22,6 +22,7 @@ import java.io.Serializable; +import javax.faces.event.ValueChangeEvent; import javax.validation.constraints.Pattern; import javax.validation.constraints.Size; @@ -63,10 +64,11 @@ public class ProfileAction implements Serializable private String username; private String activationKey; private boolean valid; + private boolean newUser; @In ApplicationConfiguration applicationConfiguration; - + @Logger Log log; @@ -93,14 +95,14 @@ public class ProfileAction implements Serializable @In RegisterService registerServiceImpl; - + @In EmailChangeService emailChangeService; private void validateEmail(String email) { HPerson person = personDAO.findByEmail(email); - + if( person != null && !person.getAccount().equals( authenticatedAccount ) ) { valid = false; @@ -108,9 +110,9 @@ private void validateEmail(String email) } } - private void validateUsername() + private void validateUsername(String username) { - HAccount account = accountDAO.getByUsername(this.username); + HAccount account = accountDAO.getByUsername(username); if( account != null && !account.equals( authenticatedAccount ) ) { @@ -119,11 +121,18 @@ private void validateUsername() } } + public void verifyUsernameAvailable(ValueChangeEvent e) + { + String username = (String) e.getNewValue(); + validateUsername(username); + } + @Create public void onCreate() { username = identity.getCredentials().getUsername(); - if (identityStore.isNewUser(username)) + newUser = identityStore.isNewUser(username); + if (newUser) { name = identity.getCredentials().getUsername(); String domain = applicationConfiguration.getDomainName(); @@ -131,7 +140,7 @@ public void onCreate() { email = ""; } - else + else { if( applicationConfiguration.isOpenIdAuth() ) { @@ -188,7 +197,7 @@ public String getUsername() public void setUsername(String username) { this.username = username; - validateUsername(); + validateUsername(username); } public String getActivationKey() @@ -206,7 +215,7 @@ public String edit() { this.valid = true; validateEmail(this.email); - validateUsername(); + validateUsername(username); if( !this.isValid() ) { @@ -267,7 +276,8 @@ public boolean isValid() public boolean isNewUser() { - return identityStore.isNewUser(username); + // in case someone else has registered in the middle + return newUser || identityStore.isNewUser(username); } } diff --git a/zanata-war/src/main/webapp/profile/edit.xhtml b/zanata-war/src/main/webapp/profile/edit.xhtml index 9fbb043c7e..1a3d7effb0 100644 --- a/zanata-war/src/main/webapp/profile/edit.xhtml +++ b/zanata-war/src/main/webapp/profile/edit.xhtml @@ -36,7 +36,8 @@ #{messages['jsf.Username']} - + From 9c85e179bd449a27ba3bc854ca3131976a7db6ff Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 9 Sep 2013 14:42:11 +1000 Subject: [PATCH 35/39] make functional test more robust --- .../src/main/java/org/zanata/page/AbstractPage.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/functional-test/src/main/java/org/zanata/page/AbstractPage.java b/functional-test/src/main/java/org/zanata/page/AbstractPage.java index d9a706105f..f4878ab4f1 100644 --- a/functional-test/src/main/java/org/zanata/page/AbstractPage.java +++ b/functional-test/src/main/java/org/zanata/page/AbstractPage.java @@ -82,11 +82,14 @@ protected void clickAndCheckErrors(WebElement button) protected void clickAndExpectErrors(WebElement button) { button.click(); - List errors = getErrors(); - if (errors.isEmpty()) + refreshPageUntil(this, new Predicate() { - throw new RuntimeException("Errors expected, none found."); - } + @Override + public boolean apply(WebDriver input) + { + return getErrors().size() > 0; + } + }); } public List getErrors() From 57f8743af6544eef197473f3efaf4826f0a35a07 Mon Sep 17 00:00:00 2001 From: Patrick Huang Date: Mon, 9 Sep 2013 13:39:48 +1000 Subject: [PATCH 36/39] rhbz1002340 - verify username availability in first openid log in --- .../src/test/resources/conf/standalone.xml | 6 ++++ .../java/org/zanata/action/ProfileAction.java | 30 ++++++++++++------- zanata-war/src/main/webapp/profile/edit.xhtml | 3 +- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/functional-test/src/test/resources/conf/standalone.xml b/functional-test/src/test/resources/conf/standalone.xml index 3f15311ac6..2d91872b6b 100644 --- a/functional-test/src/test/resources/conf/standalone.xml +++ b/functional-test/src/test/resources/conf/standalone.xml @@ -218,6 +218,7 @@ + @@ -262,6 +263,11 @@ + + + + + diff --git a/zanata-war/src/main/java/org/zanata/action/ProfileAction.java b/zanata-war/src/main/java/org/zanata/action/ProfileAction.java index 27e6e4513b..6c4e23b5c8 100644 --- a/zanata-war/src/main/java/org/zanata/action/ProfileAction.java +++ b/zanata-war/src/main/java/org/zanata/action/ProfileAction.java @@ -22,6 +22,7 @@ import java.io.Serializable; +import javax.faces.event.ValueChangeEvent; import javax.validation.constraints.Pattern; import javax.validation.constraints.Size; @@ -63,10 +64,11 @@ public class ProfileAction implements Serializable private String username; private String activationKey; private boolean valid; + private boolean newUser; @In ApplicationConfiguration applicationConfiguration; - + @Logger Log log; @@ -93,14 +95,14 @@ public class ProfileAction implements Serializable @In RegisterService registerServiceImpl; - + @In EmailChangeService emailChangeService; private void validateEmail(String email) { HPerson person = personDAO.findByEmail(email); - + if( person != null && !person.getAccount().equals( authenticatedAccount ) ) { valid = false; @@ -108,9 +110,9 @@ private void validateEmail(String email) } } - private void validateUsername() + private void validateUsername(String username) { - HAccount account = accountDAO.getByUsername(this.username); + HAccount account = accountDAO.getByUsername(username); if( account != null && !account.equals( authenticatedAccount ) ) { @@ -119,11 +121,18 @@ private void validateUsername() } } + public void verifyUsernameAvailable(ValueChangeEvent e) + { + String username = (String) e.getNewValue(); + validateUsername(username); + } + @Create public void onCreate() { username = identity.getCredentials().getUsername(); - if (identityStore.isNewUser(username)) + newUser = identityStore.isNewUser(username); + if (newUser) { name = identity.getCredentials().getUsername(); String domain = applicationConfiguration.getDomainName(); @@ -131,7 +140,7 @@ public void onCreate() { email = ""; } - else + else { if( applicationConfiguration.isOpenIdAuth() ) { @@ -188,7 +197,7 @@ public String getUsername() public void setUsername(String username) { this.username = username; - validateUsername(); + validateUsername(username); } public String getActivationKey() @@ -206,7 +215,7 @@ public String edit() { this.valid = true; validateEmail(this.email); - validateUsername(); + validateUsername(username); if( !this.isValid() ) { @@ -267,7 +276,8 @@ public boolean isValid() public boolean isNewUser() { - return identityStore.isNewUser(username); + // in case someone else has registered in the middle + return newUser || identityStore.isNewUser(username); } } diff --git a/zanata-war/src/main/webapp/profile/edit.xhtml b/zanata-war/src/main/webapp/profile/edit.xhtml index 9fbb043c7e..1a3d7effb0 100644 --- a/zanata-war/src/main/webapp/profile/edit.xhtml +++ b/zanata-war/src/main/webapp/profile/edit.xhtml @@ -36,7 +36,8 @@ #{messages['jsf.Username']} - + From c318a37d936b110c56d55441f779dffb842f7a48 Mon Sep 17 00:00:00 2001 From: Alex Eng Date: Tue, 10 Sep 2013 08:53:14 +1000 Subject: [PATCH 37/39] Fix redirect to register page after login: https://bugzilla.redhat.com/show_bug.cgi?id=981071 --- .../zanata/security/AuthenticationManager.java | 2 +- .../org/zanata/security/UserRedirectBean.java | 15 ++++++++++++--- zanata-war/src/main/webapp/WEB-INF/pages.xml | 6 ++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/security/AuthenticationManager.java b/zanata-war/src/main/java/org/zanata/security/AuthenticationManager.java index 0ace0555c4..5f8b680f99 100644 --- a/zanata-war/src/main/java/org/zanata/security/AuthenticationManager.java +++ b/zanata-war/src/main/java/org/zanata/security/AuthenticationManager.java @@ -249,7 +249,7 @@ else if (identity.isLoggedIn()) { if (userRedirect != null) { - if(userRedirect.isRedirect() && !userRedirect.isRedirectToHome()) + if(userRedirect.isRedirect() && !userRedirect.isRedirectToHome() && !userRedirect.isRedirectToRegister()) { return "redirect"; } diff --git a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java index 072db6eeb1..f4de067437 100644 --- a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java +++ b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java @@ -29,8 +29,6 @@ import org.jboss.seam.annotations.AutoCreate; import org.jboss.seam.annotations.Name; import org.jboss.seam.annotations.Scope; -import org.jboss.seam.faces.Redirect; -import org.jboss.seam.security.Identity; import org.jboss.seam.web.ServletContexts; /** @@ -51,6 +49,7 @@ public class UserRedirectBean implements Serializable { private static final String HOME_URL = "/"; + private static final String REGISTER_URL = "/register"; private static final String ERROR_URL = "/error"; /** @@ -177,6 +176,16 @@ public boolean isRedirect() } public boolean isRedirectToHome() + { + return isRedirectTo(HOME_URL); + } + + public boolean isRedirectToRegister() + { + return isRedirectTo(REGISTER_URL); + } + + private boolean isRedirectTo(String URL) { if (isRedirect()) { @@ -187,7 +196,7 @@ public boolean isRedirectToHome() url = url.substring(0, qsIndex); } - if (url.endsWith(HOME_URL)) + if (url.endsWith(URL)) { return true; } diff --git a/zanata-war/src/main/webapp/WEB-INF/pages.xml b/zanata-war/src/main/webapp/WEB-INF/pages.xml index 3e86be2a92..9dae70a0ce 100644 --- a/zanata-war/src/main/webapp/WEB-INF/pages.xml +++ b/zanata-war/src/main/webapp/WEB-INF/pages.xml @@ -56,6 +56,9 @@ + + + @@ -108,6 +111,9 @@ + + + From 5cafe655de816ab5a7d5cfa6d8edcb7c630632e6 Mon Sep 17 00:00:00 2001 From: Alex Eng Date: Tue, 10 Sep 2013 09:06:21 +1000 Subject: [PATCH 38/39] rename variables URL to url --- .../src/main/java/org/zanata/security/UserRedirectBean.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java index f4de067437..2417ecb5f2 100644 --- a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java +++ b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java @@ -189,7 +189,7 @@ private boolean isRedirectTo(String URL) { if (isRedirect()) { - String url = getUrl(); + String redirectUrl = getUrl(); int qsIndex = url.indexOf('?'); if (qsIndex > 0) { From ccc5cb92cd179d43e4d72002d47e85cc91e3ebf0 Mon Sep 17 00:00:00 2001 From: Alex Eng Date: Tue, 10 Sep 2013 09:29:17 +1000 Subject: [PATCH 39/39] Create shouldRedirectToDashboard to merge redirect pages conditions --- .../org/zanata/security/UserRedirectBean.java | 16 +++++++++++----- zanata-war/src/main/webapp/WEB-INF/pages.xml | 10 ++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java index 2417ecb5f2..cb5199ea02 100644 --- a/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java +++ b/zanata-war/src/main/java/org/zanata/security/UserRedirectBean.java @@ -185,18 +185,24 @@ public boolean isRedirectToRegister() return isRedirectTo(REGISTER_URL); } - private boolean isRedirectTo(String URL) + // provided user is logged in, they should be redirect to dashboard + public boolean shouldRedirectToDashboard() + { + return isRedirectToHome() || isRedirectToRegister(); + } + + private boolean isRedirectTo(String url) { if (isRedirect()) { - String redirectUrl = getUrl(); - int qsIndex = url.indexOf('?'); + String redirectingUrl = getUrl(); + int qsIndex = redirectingUrl.indexOf('?'); if (qsIndex > 0) { - url = url.substring(0, qsIndex); + redirectingUrl = redirectingUrl.substring(0, qsIndex); } - if (url.endsWith(URL)) + if (redirectingUrl.endsWith(url)) { return true; } diff --git a/zanata-war/src/main/webapp/WEB-INF/pages.xml b/zanata-war/src/main/webapp/WEB-INF/pages.xml index 9dae70a0ce..1a6acf775d 100644 --- a/zanata-war/src/main/webapp/WEB-INF/pages.xml +++ b/zanata-war/src/main/webapp/WEB-INF/pages.xml @@ -53,12 +53,9 @@ - + - - - @@ -108,10 +105,7 @@ - - - - +