Skip to content

Commit

Permalink
Add advanced search component to editor (ZNTA-975) (#443)
Browse files Browse the repository at this point in the history
* refactor(TransUnitFilter): linting fixes

* refactor(editor filter): move status filter data to state.phrases.filter

The status filtering is all about filtering the list of phrases, so
it belongs in the phrase section with advanced search filter data which
will soon be added.
This change will help reduce the amount of distant data that reducers need
to access.

* refactor(editor filter): move phrase status filter actions to phrases-filter-actions

This includes making the TransUnitFilter component a completely connected
component deriving all its props from mapStateToProps and mapDispatchToProps.
All the components in the headers should move in this direction, so that
the header components can stick to the single responsibility of defining
the layout of the other components.

* refactor(phrase filter): use flux standard actions for status filter

All the actions will be updated to be flux standard actions (FSAs), and
I have edited these actions, so this is an appropriate time to update
them.
This also removes the warning for using a non-symbol as an action type,
since all action types will be changed to strings so that they are
serializable (required for advanced development tools such as time-travel
debugging).

* refactor: rename TransUnitFilter to PhraseStatusFilter

* feat(ZNTA-975): add advanced search state, actions and reducer

* test: fix linting warnings in EditorSearchInput.test.js

* feat(ZNTA-975): connect EditorSearchInput to app and state

* feat(ZNTA-975): add filter fields to status list endpoint

* feat(ZNTA-975): apply style class to input component

Includes a FIXME comment because the css classes need to be moved for
maintainability.

* refactor(editor): move middleware config to separate module

* feat(ZNTA-975): add selectors for current page of phrases

These are not used yet, but selectors should replace most places that we
are manually pulling out or calculating pieces of state from the store.
They will allow changing in one place if we move some state, and will
be more efficient for calculated state since they memoize the input selectors.

* feat(ZNTA-975): allow filter parameters in phrase fetch request

* WIP: fetch filter phrases from server (NOT USED YET)

* WIP: add logging wrapper around redux-watch

* feat(ZNTA-975): fetch and store server-filtered phrase list

Does not yet display the fetched list.

* feat(ZNTA-975): add selector for hasAdvancedFilter

* feat(ZNTA-975): display filtered phrases when filter is present

* feat(ZNTA-975): WIP fixing all phrase list refs for filter, fixed paging

Paging was using old location, changed to use selectors to it will adapt
when selectors change next time.

* feat(ZNTA-975): use watcher for glossary and suggestion searches

Includes moving middleware setup to middlewares/index.js since that
makes more sense and keeps the top level cleaner.

* fix(setState): use callback form of setState for search input

The other form has a theoretical state-clobbering bug.

* feat(ZNTA-975): only select first row when unfiltered list is loaded

* test(ZNTA-975): update tests to work with plain/filtered status lists

* feat(ZNTA-975) fix advanced search icon alignment

* feat(ZNTA-975): use standard action names for phrase list and detail

* feat(ZNTA-975): use consistent names for search fields between server and editor

* feat(ZNTA-975): clean up some comments

* feat(ZNTA-975): prevent network request for empty filter search

* feat(ZNTA-975): use request body for phrase filter fields

* feat(ZNTA-975): add middleware to include meta.timestamp in all API calls

The middleware also adds some repeated defaults such as credentials and
JSON headers, which avoids the need to wrap calls in a helper function.
I chose a distinct type rather than reusing CALL_API so that there is a
way to skip the new behaviour if that is ever needed.

* feat(ZNTA-975): use timestamp to ignore stale advanced search results

* feat(ZNTA-975): correctly select visible phrase when phrases load

* feat(ZNTA-975): limit to a single phrase detail request at a time

Also only fetches phrases that do not yet have detail present.

* feat(ZNTA-975): remove some unused css

* feat(ZNTA-975): move styles for EditorSearchInput to its css file

They were in the root css file. There is still duplication and
rules conflicting/competing with other rules so more cleanup is
needed.

* feat(ZNTA-975): remove unused functions

These are replaced by new functions in the watchers, since that is
the only place they are used.

* feat(ZNTA-975) added media queries for EditorSearchInput

* feat(ZNTA-975) improved width of advanced search panel in editor

* feat(ZNTA-975) css formatting

* refactor: add FIXME comment for fragile css rule

I forgot to edit my reminder comment before and it was removed. This
replaces it with a more useful comment.

* test(ZNTA-975): update some tests to factor in request timestamps

* feat(ZNTA-975): remove blur-based closing of advanced search panel

This is less surprising to users, but the main reason for this change
is that the show/hide advanced button is confusing when the advanced
panel is visible-on-focus only. The focus tracking to have that button
toggle back to "show advanced" on blur is very complicated because it
is difficult to tell whether the activity that caused a blur came from
within the panel when it was not a focus on another input element.

* test(ZNTA-975): update test to match new expected advanced search panel behaviour

* feat(ZNTA-975): fix storybook prop names

* feat(ZNTA-975): align advanced search inputs

Also attempted to make it less visually busy by removing the
borders and using a more subtle colouring and shadow. Could
still need some work.

* test(ZNTA-975): update test to match new markup in advanced search fields

* fix(ZNTA-975): add missing Strings dependency

This was removed by accident when resolving a merge conflict.

* fix(ZNTA-975): remove some unused imports

This is to satisfy the Java compiler warnings check which is currently
marking the build unstable.

* fix(ZNTA-975): wait for phrase loading in editor test

* feat(ZNTA-975): fix advanced search input and panel width across browsers

Stretch input to fill container in firefox, and limit panel width to width
of container in chrome.

* feat(ZNTA-975): move labels for advanced search to give inputs more width

At narrow panel sizes, the inputs for advanced search fields were too narrow.
This moves the labels above the inputs so they can span the full width of the
panel.

* feat(ZNTA-975): update test for changed EditorSearchInput markup

* test: increase test coverage

* refactor: delete unused module
  • Loading branch information
davidmason committed Aug 16, 2017
1 parent 30b993a commit 3d788f8
Show file tree
Hide file tree
Showing 60 changed files with 2,213 additions and 1,423 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.zanata.rest.dto;

import com.webcohesion.enunciate.metadata.DocumentationExample;
import com.webcohesion.enunciate.metadata.Label;
import org.codehaus.jackson.map.annotate.JsonSerialize;

import java.io.Serializable;

@JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
@Label("Filter Constraints")
public class FilterFields implements Serializable {
private static final long serialVersionUID = 1L;

private String searchString;
private String resId;
private String changedBefore;
private String changedAfter;
private String lastModifiedByUser;
private String sourceComment;
private String transComment;
private String msgContext;

@DocumentationExample(value = "best of times", value2 = "besten Zeiten")
public String getSearchString() {
return searchString;
}

public void setSearchString(String searchString) {
this.searchString = searchString;
}

@DocumentationExample(value = "521bbc0112de4af32209f11f07809614", value2 = "93453e83a6707092b76ebb960faf024e")
public String getResId() {
return resId;
}

public void setResId(String resId) {
this.resId = resId;
}

@DocumentationExample(value = "2016-12-16", value2 = "2001-01-15")
public String getChangedBefore() {
return changedBefore;
}

public void setChangedBefore(String changedBefore) {
this.changedBefore = changedBefore;
}

@DocumentationExample(value = "2004-03-02", value2 = "2014-11-12")
public String getChangedAfter() {
return changedAfter;
}

public void setChangedAfter(String changedAfter) {
this.changedAfter = changedAfter;
}

@DocumentationExample(value = "damason", value2 = "jsmith")
public String getLastModifiedByUser() {
return lastModifiedByUser;
}

public void setLastModifiedByUser(String lastModifiedByUser) {
this.lastModifiedByUser = lastModifiedByUser;
}

@DocumentationExample(value = "full name of the user", value2 = "product name")
public String getSourceComment() {
return sourceComment;
}

public void setSourceComment(String sourceComment) {
this.sourceComment = sourceComment;
}

@DocumentationExample(value = "translated literally", value2 = "did not translate")
public String getTransComment() {
return transComment;
}

public void setTransComment(String transComment) {
this.transComment = transComment;
}

@DocumentationExample(value = "main.c", value2 = "users.js")
public String getMsgContext() {
return msgContext;
}

public void setMsgContext(String msgContext) {
this.msgContext = msgContext;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@
import javax.ws.rs.DefaultValue;
import javax.ws.rs.GET;
import javax.ws.rs.HEAD;
import javax.ws.rs.POST;
import javax.ws.rs.PUT;
import javax.ws.rs.Path;
import javax.ws.rs.PathParam;
import javax.ws.rs.Produces;

import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;

import com.webcohesion.enunciate.metadata.rs.ResourceLabel;
import org.zanata.rest.MediaTypes;
import org.zanata.rest.dto.FilterFields;
import org.zanata.rest.dto.LocaleDetails;
import org.zanata.rest.dto.ProjectIteration;
import org.zanata.rest.dto.TransUnitStatus;
Expand Down Expand Up @@ -239,8 +242,10 @@ public Response getDocuments(@PathParam("projectSlug") String projectSlug,
@PathParam("versionSlug") String versionSlug);

/**
* Retrieves a list translation unit with status in a document.
* Queries for a list of translation unit id with status in a document.
*
* @param filterConstraints
* Optional filtering based on one or several fields.
* @param projectSlug
* Project identifier
* @param versionSlug
Expand All @@ -259,15 +264,16 @@ public Response getDocuments(@PathParam("projectSlug") String projectSlug,
* the server while performing this operation.
*
*/
@GET
@POST
@Produces({ MediaTypes.APPLICATION_ZANATA_TRANS_UNIT_RESOURCE_JSON,
MediaType.APPLICATION_JSON })
@Consumes({ MediaType.APPLICATION_JSON })
@Path(VERSION_SLUG_TEMPLATE + "/doc/{docId}/status/{localeId}")
@TypeHint(TransUnitStatus[].class)
public Response getTransUnitStatus(
@PathParam("projectSlug") String projectSlug,
@PathParam("versionSlug") String versionSlug,
@PathParam("docId") String docId,
@DefaultValue("en-US") @PathParam("localeId") String localeId);

@PathParam("projectSlug") String projectSlug,
@PathParam("versionSlug") String versionSlug,
@PathParam("docId") String docId,
@DefaultValue("en-US") @PathParam("localeId") String localeId,
FilterFields filterFields);
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,9 @@ public List<WebElement> getTransunitTargets() {
private List<WebElement> getTextUnits() {
return getDriver().findElements(transUnitText);
}

public void expectNumberOfTargets(int expected) {
waitForAMoment().until(it ->
getTransunitTargets().size() == expected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public void showAlphaEditor() {
.switchToEditorWindow();

assertThat(reactEditorPage.isReactEditor());

// Wait for the phrases to load
reactEditorPage.expectNumberOfTargets(1);
assertThat(reactEditorPage.getTransunitTargets().size()).isEqualTo(1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.zanata.common.EntityStatus.ACTIVE;
import static org.zanata.common.EntityStatus.OBSOLETE;
import static org.zanata.common.EntityStatus.READONLY;
import static org.zanata.util.DateUtil.parseQueryDate;
import static org.zanata.webtrans.server.rpc.GetTransUnitsNavigationService.TextFlowResultTransformer;

import java.util.ArrayList;
Expand Down Expand Up @@ -45,6 +46,7 @@
import org.zanata.rest.NoSuchEntityException;
import org.zanata.rest.ReadOnlyEntityException;
import org.zanata.rest.RestUtil;
import org.zanata.rest.dto.FilterFields;
import org.zanata.rest.dto.LocaleDetails;
import org.zanata.rest.dto.ProcessStatus;
import org.zanata.rest.dto.ProjectIteration;
Expand Down Expand Up @@ -303,7 +305,8 @@ public Response getTransUnitStatus(
@PathParam("projectSlug") String projectSlug,
@PathParam("versionSlug") String versionSlug,
@PathParam("docId") String noSlashDocId,
@DefaultValue("en-US") @PathParam("localeId") String localeId) {
@DefaultValue("en-US") @PathParam("localeId") String localeId,
FilterFields filterFields) {
if (StringUtils.isEmpty(noSlashDocId)) {
return Response.status(Response.Status.NOT_FOUND).build();
}
Expand All @@ -319,18 +322,29 @@ public Response getTransUnitStatus(
}
TextFlowResultTransformer resultTransformer =
new TextFlowResultTransformer(hLocale);
FilterConstraints filterConstraints =
FilterConstraints.builder().build();

FilterConstraints.Builder filterConstraints = FilterConstraints.builder();
if (filterFields != null) {
filterConstraints
.filterBy(filterFields.getSearchString())
.resourceIdIs(filterFields.getResId())
.targetChangedBefore(parseQueryDate(filterFields.getChangedBefore()))
.targetChangedAfter(parseQueryDate(filterFields.getChangedAfter()))
.lastModifiedBy(filterFields.getLastModifiedByUser())
.sourceCommentContains(filterFields.getSourceComment())
.targetCommentContains(filterFields.getTransComment())
.msgContext(filterFields.getMsgContext());
}

List<HTextFlow> textFlows = textFlowDAO.getNavigationByDocumentId(
new DocumentId(document.getId(), document.getDocId()), hLocale,
resultTransformer, filterConstraints);
resultTransformer, filterConstraints.build());
List<TransUnitStatus> statusList =
Lists.newArrayListWithExpectedSize(textFlows.size());
for (HTextFlow textFlow : textFlows) {
ContentState state =
textFlow.getTargets().get(hLocale.getId()).getState();
statusList.add(new TransUnitStatus(textFlow.getId(),
textFlow.getResId(), state));
statusList.add(new TransUnitStatus(textFlow.getId(), textFlow.getResId(), state));
}
Object entity = new GenericEntity<List<TransUnitStatus>>(statusList){};
return Response.ok(entity).build();
Expand Down
17 changes: 17 additions & 0 deletions server/services/src/main/java/org/zanata/util/DateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import java.util.Date;
import java.util.Locale;
import com.google.common.base.Strings;
import org.joda.time.DateTime;
import org.joda.time.Days;
import org.joda.time.Period;
Expand All @@ -21,6 +22,8 @@ public class DateUtil {

private static final String DATE_TIME_SHORT_PATTERN = "dd/MM/yy HH:mm";
private static final String TIME_SHORT_PATTERN = "hh:mm:ss";
// Used for advanced editor search queries
private static final String DATE_SHORT_QUERY_PATTERN = "yyyy-MM-dd";
// Period Formatters are thread safe and immutable according to joda time
// docs
private static final PeriodFormatter TIME_REMAINING_FORMATTER =
Expand Down Expand Up @@ -231,4 +234,18 @@ public static boolean isDatesInRange(Date from, Date to, int days) {
Days d = Days.daysBetween(fromDate, toDate);
return d.getDays() <= days;
}

/**
* Parse a yyyy-mm-dd string to a date.
*
* @param dateString in form "yyyy-mm-dd" or empty string or null
* @return the parsed date or null.
*/
public static DateTime parseQueryDate(String dateString) {
if (Strings.isNullOrEmpty(dateString)) {
return null;
}
return DateTimeFormat.forPattern(DATE_SHORT_QUERY_PATTERN)
.parseDateTime(dateString);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@
import javax.enterprise.context.RequestScoped;
import javax.inject.Inject;
import javax.inject.Named;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;
import org.zanata.dao.TextFlowDAO;
import org.zanata.exception.ZanataServiceException;
import org.zanata.model.HLocale;
Expand All @@ -37,6 +34,7 @@
import org.zanata.security.ZanataIdentity;
import org.zanata.service.LocaleService;
import org.zanata.service.ValidationService;
import static org.zanata.util.DateUtil.parseQueryDate;
import org.zanata.webtrans.server.ActionHandlerFor;
import org.zanata.webtrans.shared.model.TransUnit;
import org.zanata.webtrans.shared.rpc.EditorFilter;
Expand All @@ -46,7 +44,6 @@
import org.zanata.webtrans.shared.rpc.GetTransUnitsNavigationResult;
import org.zanata.webtrans.shared.util.FindByTransUnitIdPredicate;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;

Expand All @@ -70,8 +67,6 @@ public class GetTransUnitListHandler extends
private ValidationService validationServiceImpl;
@Inject
private GetTransUnitsNavigationService getTransUnitsNavigationService;
private DateTimeFormatter dateFormatter =
DateTimeFormat.forPattern("yyyy-MM-dd");

@Override
public GetTransUnitListResult execute(GetTransUnitList action,
Expand All @@ -86,10 +81,10 @@ public GetTransUnitListResult execute(GetTransUnitList action,
FilterConstraints constraints = FilterConstraints.builder()
.filterBy(editorFilter.getTextInContent())
.lastModifiedBy(editorFilter.getLastModifiedByUser())
.targetChangedBefore(parseDateIfPresent(
.targetChangedBefore(parseQueryDate(
editorFilter.getLastModifiedBefore()))
.targetChangedAfter(
parseDateIfPresent(editorFilter.getLastModifiedAfter()))
parseQueryDate(editorFilter.getLastModifiedAfter()))
.resourceIdIs(editorFilter.getResId())
.msgContext(editorFilter.getMsgContext())
.sourceCommentContains(editorFilter.getSourceComment())
Expand Down Expand Up @@ -127,11 +122,6 @@ public GetTransUnitListResult execute(GetTransUnitList action,
return result;
}

private DateTime parseDateIfPresent(String dateInString) {
return Strings.isNullOrEmpty(dateInString) ? null
: dateFormatter.parseDateTime(dateInString);
}

private int getTotalPageIndex(int indexListSize, int countPerPage) {
int totalPageNumber =
(int) Math.ceil((float) indexListSize / countPerPage);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public void getTransUnitStatusWillReturnNotFoundIfDocumentNotFound() {
.thenReturn(null);

Response response =
service.getTransUnitStatus("a", "1", "authors", "de");
service.getTransUnitStatus("a", "1", "authors", "de", null);
assertThat(response.getStatus()).isEqualTo(404);
}

Expand All @@ -249,7 +249,7 @@ public void getTransUnitStatusWillReturnNotFoundIfLocaletNotFound() {
when(localeService.getByLocaleId("de")).thenReturn(null);

Response response =
service.getTransUnitStatus("a", "1", "authors", "de");
service.getTransUnitStatus("a", "1", "authors", "de", null);
assertThat(response.getStatus()).isEqualTo(404);
}

Expand All @@ -261,7 +261,7 @@ public void getTransUnitStatusWillGetResults() {
new HLocale(LocaleId.DE));

Response response =
service.getTransUnitStatus("a", "1", "authors", "de");
service.getTransUnitStatus("a", "1", "authors", "de", null);
assertThat(response.getStatus()).isEqualTo(200);
verify(textFlowDAO).getNavigationByDocumentId(isA(DocumentId.class), isA(HLocale.class), isA(
GetTransUnitsNavigationService.TextFlowResultTransformer.class), isA(FilterConstraints.class));
Expand Down
Loading

0 comments on commit 3d788f8

Please sign in to comment.