diff --git a/CODEOWNERS b/CODEOWNERS index af182e2..7d8aae4 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @FortnoxAB/rocket-fuelers +* @FortnoxAB/rocket-fuelers diff --git a/api/src/main/java/api/Answer.java b/api/src/main/java/api/Answer.java index 9c19baa..9c48ff2 100644 --- a/api/src/main/java/api/Answer.java +++ b/api/src/main/java/api/Answer.java @@ -1,8 +1,6 @@ package api; import java.time.LocalDateTime; -import java.time.ZonedDateTime; -import java.util.Objects; import static java.util.Objects.nonNull; diff --git a/api/src/main/java/api/Post.java b/api/src/main/java/api/Post.java index 091ba9e..3b49bad 100644 --- a/api/src/main/java/api/Post.java +++ b/api/src/main/java/api/Post.java @@ -1,5 +1,6 @@ package api; + import java.time.LocalDateTime; /** * A class that defines the shared attributes between a {@link Answer} @@ -14,7 +15,7 @@ public abstract class Post { private String picture; - private String createdAt; + private LocalDateTime createdAt; private Integer votes; @@ -30,11 +31,11 @@ public void setCreatedBy(String createdBy) { this.createdBy = createdBy; } - public String getCreatedAt() { + public LocalDateTime getCreatedAt() { return createdAt; } - public void setCreatedAt(String createdAt) { + public void setCreatedAt(LocalDateTime createdAt) { this.createdAt = createdAt; } diff --git a/api/src/main/java/api/QuestionResource.java b/api/src/main/java/api/QuestionResource.java index d849a91..391f9ef 100644 --- a/api/src/main/java/api/QuestionResource.java +++ b/api/src/main/java/api/QuestionResource.java @@ -2,6 +2,7 @@ import api.auth.Auth; import rx.Observable; +import se.fortnox.reactivewizard.CollectionOptions; import javax.ws.rs.DELETE; import javax.ws.rs.GET; @@ -33,14 +34,44 @@ public interface QuestionResource { Observable getQuestion(Auth auth, @PathParam("questionId") long questionId); /** - * Return a list of latest questions with a limit + * Return a list of latest questions * - * @param limit + * @param options Sorting and limiting options * @return questions */ @GET @Path("questions/latest") - Observable> getLatestQuestion(@QueryParam("limit") Integer limit); + Observable> getLatestQuestions(CollectionOptions options); + + /** + * Return a list of the highest voted questions + * + * @param options Sorting and limiting options + * @return questions + */ + @GET + @Path("questions/popular") + Observable> getPopularQuestions(CollectionOptions options); + + /** + * Return a list of the highest voted questions without any answers + * + * @param options Sorting and limiting options + * @return questions + */ + @GET + @Path("questions/popularunanswered") + Observable> getPopularUnansweredQuestions(CollectionOptions options); + + /** + * Return a list of the questions that had an answer accepted the most recently + * + * @param options Sorting and limiting options + * @return questions + */ + @GET + @Path("questions/recentlyaccepted") + Observable> getRecentlyAcceptedQuestions(CollectionOptions options); /** * Adds a question and links it to the given userId. @@ -54,7 +85,7 @@ public interface QuestionResource { */ @GET @Path("questions") - Observable> getQuestionsBySearchQuery(@QueryParam("search") String searchQuery, @QueryParam("limit") Integer limit); + Observable> getQuestionsBySearchQuery(@QueryParam("search") String searchQuery, CollectionOptions options); /** @@ -62,7 +93,7 @@ public interface QuestionResource { */ @Path("users/{userId}/questions") @GET - Observable> getQuestions(@PathParam("userId") long userId); + Observable> getQuestions(@PathParam("userId") long userId, CollectionOptions options); /** * Updates the question with the given questionId diff --git a/impl/src/main/java/dao/QuestionDao.java b/impl/src/main/java/dao/QuestionDao.java index 2d32661..f43eb2c 100644 --- a/impl/src/main/java/dao/QuestionDao.java +++ b/impl/src/main/java/dao/QuestionDao.java @@ -2,52 +2,142 @@ import api.Question; import rx.Observable; +import se.fortnox.reactivewizard.CollectionOptions; import se.fortnox.reactivewizard.db.GeneratedKey; import se.fortnox.reactivewizard.db.Query; import se.fortnox.reactivewizard.db.Update; +import java.time.LocalDateTime; + public interface QuestionDao { @Query( + value = "SELECT " + "question.id, " + "question.question, " + "answer_accepted, " + "question.title, " + "question.bounty, " + - "question.votes, " + "question.created_at, " + - "question,user_id, " + + "question.user_id, " + "question.slack_id, \"user\".picture, \"user\".name as created_by, " + "(SELECT COALESCE(SUM(question_vote.value), 0) FROM question_vote WHERE question_vote.question_id = question.id) AS votes " + "FROM " + "question " + "INNER JOIN " + - "\"user\" on \"user\".id = question.user_id WHERE question.user_id=:userId") - Observable getQuestions(long userId); + "\"user\" on \"user\".id = question.user_id WHERE question.user_id=:userId " + + "ORDER BY " + + "question.created_at DESC ", + defaultLimit = 10, + maxLimit = 50) + Observable getQuestions(long userId, CollectionOptions options); @Query( + value = "SELECT " + "question.id, " + "question.question, " + "answer_accepted, " + "question.title, " + "question.bounty, " + - "question.votes, " + "question.created_at, " + - "question,user_id, " + + "question.user_id, " + "question.slack_id, " + - "\"user\".picture, \"user\".name as created_by, " + + "\"user\".picture, " + + "\"user\".name as created_by, " + "(SELECT COALESCE(SUM(question_vote.value), 0) FROM question_vote WHERE question_vote.question_id = question.id) AS votes " + "FROM " + "question " + "INNER JOIN " + "\"user\" on \"user\".id = question.user_id " + "ORDER BY " + - "question.created_at DESC " + - "LIMIT " + - ":limit") - Observable getLatestQuestions(Integer limit); + "question.created_at DESC ", + defaultLimit = 10, + maxLimit = 50) + Observable getLatestQuestions(CollectionOptions options); + + @Query( + value = + "SELECT " + + "question.id, " + + "question.question, " + + "answer_accepted, " + + "question.title, " + + "question.bounty, " + + "question.created_at, " + + "question.user_id, " + + "question.slack_id, " + + "\"user\".picture, " + + "\"user\".name as created_by, " + + "(SELECT COALESCE(SUM(question_vote.value), 0) FROM question_vote WHERE question_vote.question_id = question.id) AS votes " + + "FROM " + + "question " + + "INNER JOIN " + + "\"user\" on \"user\".id = question.user_id " + + "ORDER BY " + + "votes DESC NULLS LAST, " + + "question.created_at DESC ", + defaultLimit = 10, + maxLimit = 50) + Observable getPopularQuestions(CollectionOptions options); + + @Query( + value = + "SELECT " + + "question.id, " + + "question.question, " + + "answer_accepted, " + + "question.title, " + + "question.bounty, " + + "question.created_at, " + + "question.user_id, " + + "question.slack_id, " + + "\"user\".picture, " + + "\"user\".name as created_by, " + + "(SELECT COALESCE(SUM(question_vote.value), 0) FROM question_vote WHERE question_vote.question_id = question.id) AS votes " + + "FROM " + + "question " + + "INNER JOIN " + + "\"user\" on \"user\".id = question.user_id " + + "LEFT JOIN " + + "answer on answer.question_id = question.id " + + "WHERE " + + "answer IS NULL " + + "ORDER BY " + + "votes DESC NULLS LAST, " + + "question.created_at DESC ", + defaultLimit = 10, + maxLimit = 50) + Observable getPopularUnansweredQuestions(CollectionOptions options); + + @Query( + value = + "SELECT " + + "question.id, " + + "question.question, " + + "answer_accepted, " + + "question.title, " + + "question.bounty, " + + "question.created_at, " + + "question.user_id, " + + "question.slack_id, " + + "\"user\".picture, " + + "\"user\".name as created_by, " + + "(SELECT COALESCE(SUM(question_vote.value), 0) FROM question_vote WHERE question_vote.question_id = question.id) AS votes " + + "FROM " + + "question " + + "INNER JOIN " + + "\"user\" on \"user\".id = question.user_id " + + "LEFT JOIN " + + "answer on answer.question_id = question.id " + + "WHERE " + + "answer.accepted_at IS NOT NULL " + + "ORDER BY " + + "answer.accepted_at DESC NULLS LAST ", + defaultLimit = 10, + maxLimit = 50) + Observable getRecentlyAcceptedQuestions(CollectionOptions options); @Update( "INSERT INTO " + @@ -55,7 +145,6 @@ public interface QuestionDao { "question, " + "title, " + "bounty, " + - "votes, " + "created_at, " + "user_id, " + "slack_id) " + @@ -64,7 +153,6 @@ public interface QuestionDao { ":question.question, " + ":question.title, " + ":question.bounty, " + - "0, " + "NOW(), " + ":userId, " + ":question.slackId" + @@ -82,7 +170,6 @@ public interface QuestionDao { "question, " + "title, " + "bounty, " + - "votes, " + "answer_accepted, " + "created_at, " + "\"user\".name as created_by, " + @@ -107,7 +194,6 @@ public interface QuestionDao { "question, " + "title, " + "bounty, " + - "votes, " + "answer_accepted, " + "created_at, " + "user_id, " + @@ -137,7 +223,6 @@ public interface QuestionDao { "SELECT " + "id, question, " + "title, bounty, " + - "votes, " + "answer_accepted, " + "created_at, user_id, " + "slack_id, " + @@ -155,13 +240,13 @@ public interface QuestionDao { Observable deleteQuestion(long userId, long questionId); @Query( + value = "SELECT DISTINCT " + "question.id, " + "question.question, " + "answer_accepted, " + "question.title, " + "question.bounty, " + - "question.votes, " + "question.created_at, " + "question, " + "question.user_id, " + @@ -180,9 +265,9 @@ public interface QuestionDao { "OR " + "answer.answer ILIKE ('%' || :search || '%') " + "ORDER BY " + - "question.votes desc, " + - "question.created_at desc " + - "LIMIT " + - ":limit") - Observable getQuestions(String search, Integer limit); + "votes desc, " + + "question.created_at desc ", + defaultLimit = 50, + maxLimit = 50) + Observable getQuestions(String search, CollectionOptions options); } diff --git a/impl/src/main/java/dates/RWDateFormat.java b/impl/src/main/java/dates/RWDateFormat.java new file mode 100644 index 0000000..65b3311 --- /dev/null +++ b/impl/src/main/java/dates/RWDateFormat.java @@ -0,0 +1,35 @@ +package dates; + +import com.fasterxml.jackson.databind.util.StdDateFormat; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.Locale; +import java.util.TimeZone; + +/** + * Workaround for framework bug. Should be removed once fixed. + */ +public class RWDateFormat extends StdDateFormat { + + public RWDateFormat() { + super(TimeZone.getDefault(), Locale.getDefault(), true); + } + + @Override + public Date parse(String dateStr) throws ParseException { + try { + return super.parse(dateStr); + } catch (ParseException e) { + SimpleDateFormat fmt = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); + fmt.setTimeZone(TimeZone.getDefault()); + return fmt.parse(dateStr); + } + } + + @Override + public StdDateFormat clone() { + return new RWDateFormat(); + } +} diff --git a/impl/src/main/java/impl/QuestionResourceImpl.java b/impl/src/main/java/impl/QuestionResourceImpl.java index 558c4be..faf17dd 100644 --- a/impl/src/main/java/impl/QuestionResourceImpl.java +++ b/impl/src/main/java/impl/QuestionResourceImpl.java @@ -11,11 +11,11 @@ import dao.QuestionDao; import dao.QuestionVote; import dao.QuestionVoteDao; -import io.netty.handler.codec.http.HttpResponseStatus; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import rx.Observable; import rx.functions.Func1; +import se.fortnox.reactivewizard.CollectionOptions; import se.fortnox.reactivewizard.jaxrs.WebException; import slack.SlackConfig; import slack.SlackResource; @@ -39,25 +39,30 @@ public class QuestionResourceImpl implements QuestionResource { private static final Logger LOG = LoggerFactory.getLogger(QuestionResourceImpl.class); - public static final String FAILED_TO_SEARCH_FOR_QUESTIONS = "failed.to.search.for.questions"; - public static final String QUESTION_NOT_FOUND = "question.not.found"; - public static final String FAILED_TO_GET_QUESTIONS_FROM_DATABASE = "failed.to.get.questions.from.database"; - public static final String FAILED_TO_GET_QUESTION_FROM_DATABASE = "failed.to.get.question.from.database"; - public static final String FAILED_TO_UPDATE_QUESTION_TO_DATABASE = "failed.to.update.question.to.database"; - public static final String NOT_OWNER_OF_QUESTION = "not.owner.of.question"; - public static final String FAILED_TO_DELETE_QUESTION = "failed.to.delete.question"; - public static final String INVALID_VOTE = "invalid.vote"; - - - private final QuestionDao questionDao; - private final QuestionVoteDao questionVoteDao; - private final SlackResource slackResource; - private final SlackConfig slackConfig; + public static final String FAILED_TO_SEARCH_FOR_QUESTIONS = "failed.to.search.for.questions"; + public static final String QUESTION_NOT_FOUND = "question.not.found"; + public static final String FAILED_TO_GET_QUESTIONS_FROM_DATABASE = "failed.to.get.questions.from.database"; + public static final String FAILED_TO_GET_QUESTION_FROM_DATABASE = "failed.to.get.question.from.database"; + public static final String FAILED_TO_UPDATE_QUESTION_TO_DATABASE = "failed.to.update.question.to.database"; + public static final String NOT_OWNER_OF_QUESTION = "not.owner.of.question"; + public static final String FAILED_TO_DELETE_QUESTION = "failed.to.delete.question"; + public static final String INVALID_VOTE = "invalid.vote"; + public static final String FAILED_TO_GET_LATEST_QUESTIONS = "failed.to.get.latest.questions"; + public static final String FAILED_TO_GET_POPULAR_QUESTIONS = "failed.to.get.popular.questions"; + public static final String FAILED_TO_GET_POPULAR_UNANSWERED_QUESTIONS = "failed.to.get.popular.unanswered.questions"; + public static final String FAILED_TO_GET_RECENTLY_ACCEPTED_QUESTIONS = "failed.to.get.recently.accepted.questions"; + public static final String FAILED_TO_ADD_QUESTION_TO_DATABASE = "failed.to.add.question.to.database"; + + private final QuestionDao questionDao; + private final QuestionVoteDao questionVoteDao; + private final SlackResource slackResource; + private final SlackConfig slackConfig; private final ApplicationConfig applicationConfig; @Inject public QuestionResourceImpl(QuestionDao questionDao, QuestionVoteDao questionVoteDao, - SlackResource slackResource, SlackConfig slackConfig, ApplicationConfig applicationConfig) { + SlackResource slackResource, SlackConfig slackConfig, ApplicationConfig applicationConfig + ) { this.questionDao = questionDao; this.questionVoteDao = questionVoteDao; this.slackResource = slackResource; @@ -68,14 +73,14 @@ public QuestionResourceImpl(QuestionDao questionDao, QuestionVoteDao questionVot @Override public Observable getQuestionBySlackThreadId(String slackThreadId) { return this.questionDao.getQuestionBySlackThreadId(slackThreadId).switchIfEmpty( - exception(() -> new WebException(HttpResponseStatus.NOT_FOUND, "not.found"))); + exception(() -> new WebException(NOT_FOUND, QUESTION_NOT_FOUND))); } - @Override - public Observable getQuestionById(long questionId) { - return this.questionDao.getQuestion(questionId).switchIfEmpty( - exception(() -> new WebException(HttpResponseStatus.NOT_FOUND, "not.found"))); - } + @Override + public Observable getQuestionById(long questionId) { + return this.questionDao.getQuestion(questionId).switchIfEmpty( + exception(() -> new WebException(NOT_FOUND, QUESTION_NOT_FOUND))); + } @Override public Observable getQuestion(Auth auth, long questionId) { @@ -87,19 +92,29 @@ public Observable getQuestion(Auth auth, long questionId) { } @Override - public Observable> getLatestQuestion(Integer limit) { - if (limit == null) { - limit = 10; - } - return this.questionDao.getLatestQuestions(limit).toList().onErrorResumeNext(e -> - error(new WebException(HttpResponseStatus.INTERNAL_SERVER_ERROR, "failed.to.get.latest.questions", e)) - ); + public Observable> getLatestQuestions(CollectionOptions options) { + return handleError(questionDao.getLatestQuestions(options), FAILED_TO_GET_LATEST_QUESTIONS); + } + + @Override + public Observable> getPopularQuestions(CollectionOptions options) { + return handleError(questionDao.getPopularQuestions(options), FAILED_TO_GET_POPULAR_QUESTIONS); + } + + @Override + public Observable> getPopularUnansweredQuestions(CollectionOptions options) { + return handleError(questionDao.getPopularUnansweredQuestions(options), FAILED_TO_GET_POPULAR_UNANSWERED_QUESTIONS); + } + + @Override + public Observable> getRecentlyAcceptedQuestions(CollectionOptions options) { + return handleError(questionDao.getRecentlyAcceptedQuestions(options), FAILED_TO_GET_RECENTLY_ACCEPTED_QUESTIONS); } @Override public Observable createQuestion(Auth auth, Question question) { return this.questionDao - .addQuestion(auth.getUserId(), question) + .addQuestion(auth.getUserId(), question) .map(longGeneratedKey -> { question.setId(longGeneratedKey.getKey()); return question; @@ -111,7 +126,7 @@ public Observable createQuestion(Auth auth, Question question) { return empty(); }); return first(postMessageToSlack).thenReturn(savedQuestion); - }).onErrorResumeNext(throwable -> error(new WebException(HttpResponseStatus.INTERNAL_SERVER_ERROR, "failed.to.add.question.to.database", throwable))); + }).onErrorResumeNext(throwable -> error(new WebException(INTERNAL_SERVER_ERROR, FAILED_TO_ADD_QUESTION_TO_DATABASE, throwable))); } private List notificationMessage(Question question) { @@ -124,7 +139,7 @@ private List notificationMessage(Question question) { } private String questionUrl(long questionId) { - return "<" + applicationConfig.getBaseUrl() + "/question/" + questionId + "|rocket-fuel>"; + return "<" + applicationConfig.getBaseUrl() + "/question/" + questionId + "|rocket-fuel>"; } private static MarkdownTextObject markdownText(String string, String... args) { @@ -134,26 +149,20 @@ private static MarkdownTextObject markdownText(String string, String... args) { } @Override - public Observable> getQuestionsBySearchQuery(String searchQuery, Integer limit) { - if (limit == null) { - limit = 50; - } + public Observable> getQuestionsBySearchQuery(String searchQuery, CollectionOptions options) { if (isNullOrEmpty(searchQuery)) { return just(emptyList()); } - return questionDao.getQuestions(searchQuery, limit) + return questionDao.getQuestions(searchQuery, options) .onErrorResumeNext(e -> { LOG.error("failed to search for questions with search query: [" + searchQuery + "]"); - return error(new WebException(HttpResponseStatus.INTERNAL_SERVER_ERROR, FAILED_TO_SEARCH_FOR_QUESTIONS ,e)); + return error(new WebException(INTERNAL_SERVER_ERROR, FAILED_TO_SEARCH_FOR_QUESTIONS, e)); }).toList(); } @Override - public Observable> getQuestions(long userId) { - return this.questionDao - .getQuestions(userId).toList() - .onErrorResumeNext(throwable -> - error(new WebException(INTERNAL_SERVER_ERROR, FAILED_TO_GET_QUESTIONS_FROM_DATABASE, throwable))); + public Observable> getQuestions(long userId, CollectionOptions options) { + return handleError(questionDao.getQuestions(userId, options), FAILED_TO_GET_QUESTIONS_FROM_DATABASE); } @Override @@ -227,5 +236,10 @@ private Func1> validateVoteAndRemoveIfZer }; } - + private static Observable> handleError(Observable questions, String errorCode) { + return questions.toList() + .onErrorResumeNext(e -> + error(new WebException(INTERNAL_SERVER_ERROR, errorCode, e)) + ); + } } diff --git a/impl/src/main/java/jaxrs/CollectionOptionsRequestParameterSerializer.java b/impl/src/main/java/jaxrs/CollectionOptionsRequestParameterSerializer.java new file mode 100644 index 0000000..d182452 --- /dev/null +++ b/impl/src/main/java/jaxrs/CollectionOptionsRequestParameterSerializer.java @@ -0,0 +1,29 @@ +package jaxrs; + +import se.fortnox.reactivewizard.CollectionOptions; +import se.fortnox.reactivewizard.client.RequestBuilder; +import se.fortnox.reactivewizard.client.RequestParameterSerializer; + +import javax.inject.Singleton; + +@Singleton +public class CollectionOptionsRequestParameterSerializer implements RequestParameterSerializer { + @Override + public void addParameter(CollectionOptions param, RequestBuilder request) { + if (param == null) { + return; + } + if (param.getLimit() != null) { + request.addQueryParam("limit", String.valueOf(param.getLimit())); + } + if (param.getOffset() != null) { + request.addQueryParam("offset", String.valueOf(param.getOffset())); + } + if (param.getSortBy() != null) { + request.addQueryParam("sortby", param.getSortBy()); + } + if (param.getOrder() != null) { + request.addQueryParam("order", param.getOrder().name()); + } + } +} diff --git a/impl/src/main/java/jaxrs/CollectionOptionsResolver.java b/impl/src/main/java/jaxrs/CollectionOptionsResolver.java new file mode 100644 index 0000000..7ff3267 --- /dev/null +++ b/impl/src/main/java/jaxrs/CollectionOptionsResolver.java @@ -0,0 +1,41 @@ +package jaxrs; + +import rx.Observable; +import se.fortnox.reactivewizard.CollectionOptions; +import se.fortnox.reactivewizard.db.paging.CollectionOptionsWithResult; +import se.fortnox.reactivewizard.jaxrs.JaxRsRequest; +import se.fortnox.reactivewizard.jaxrs.params.ParamResolver; + +import static rx.Observable.just; + +public class CollectionOptionsResolver implements ParamResolver { + + @Override + public Observable resolve(JaxRsRequest request) { + return just(new CollectionOptionsWithResult(getQueryParamAsInteger(request, "limit"), + getQueryParamAsInteger(request, "offset"), + request.getQueryParam("sortby"), + getQueryParamAsSortOrder(request, "order"))); + } + + private CollectionOptions.SortOrder getQueryParamAsSortOrder(JaxRsRequest request, String key) { + String order = request.getQueryParam(key); + if (order != null) { + return CollectionOptions.SortOrder.valueOf(order.toUpperCase()); + } else { + return null; + } + } + + private Integer getQueryParamAsInteger(JaxRsRequest request, String key) { + String val = request.getQueryParam(key); + if (val == null) { + return null; + } + try { + return Integer.valueOf(val); + } catch (NumberFormatException e) { + return null; + } + } +} diff --git a/impl/src/main/java/jaxrs/JaxRsModule.java b/impl/src/main/java/jaxrs/JaxRsModule.java new file mode 100644 index 0000000..a47d3e2 --- /dev/null +++ b/impl/src/main/java/jaxrs/JaxRsModule.java @@ -0,0 +1,81 @@ +package jaxrs; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; +import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateDeserializer; +import com.fasterxml.jackson.datatype.jsr310.deser.LocalDateTimeDeserializer; +import com.google.inject.Binder; +import com.google.inject.TypeLiteral; +import com.google.inject.multibindings.Multibinder; +import dates.RWDateFormat; +import se.fortnox.reactivewizard.binding.AutoBindModule; +import se.fortnox.reactivewizard.binding.scanners.InjectAnnotatedScanner; +import se.fortnox.reactivewizard.client.RequestParameterSerializer; +import se.fortnox.reactivewizard.jaxrs.params.ParamResolver; + +import javax.inject.Inject; +import java.text.DateFormat; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; + +import static com.fasterxml.jackson.databind.DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT; +import static com.fasterxml.jackson.databind.DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT; +import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES; +import static com.fasterxml.jackson.databind.SerializationFeature.WRITE_DATES_AS_TIMESTAMPS; +import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; +import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; + +/** + * Workaround for framework bug. Should be removed once fixed. + */ +public class JaxRsModule implements AutoBindModule { + + private final InjectAnnotatedScanner injectAnnotatedScanner; + + @Inject + public JaxRsModule(InjectAnnotatedScanner injectAnnotatedScanner) { + this.injectAnnotatedScanner = injectAnnotatedScanner; + } + + @Override + public void configure(Binder binder) { + binder.bind(DateFormat.class).toProvider(RWDateFormat::new); + binder.bind(ObjectMapper.class).toInstance(new ObjectMapper() + .findAndRegisterModules() + .registerModule(createJavaTimeModule()) + .configure(WRITE_DATES_AS_TIMESTAMPS, false) + .configure(FAIL_ON_UNKNOWN_PROPERTIES, false) + .configure(ACCEPT_EMPTY_STRING_AS_NULL_OBJECT, true) + .configure(ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT, true) + .setDateFormat(new RWDateFormat())); + Multibinder.newSetBinder(binder, TypeLiteral.get(ParamResolver.class)) + .addBinding() + .to(CollectionOptionsResolver.class); + Multibinder.newSetBinder(binder, TypeLiteral.get(RequestParameterSerializer.class)) + .addBinding() + .to(CollectionOptionsRequestParameterSerializer.class); + } + + private static JavaTimeModule createJavaTimeModule() { + JavaTimeModule javaTimeModule = new JavaTimeModule(); + final DateTimeFormatter dateTimeFormatterAllowingSpace = new DateTimeFormatterBuilder().parseCaseInsensitive() + .append(ISO_LOCAL_DATE) + .optionalStart() + .appendPattern("[[ ]['T']]") + .append(ISO_LOCAL_TIME) + .optionalEnd() + .toFormatter(); + javaTimeModule.addDeserializer(LocalDateTime.class, new LocalDateTimeDeserializer(dateTimeFormatterAllowingSpace)); + javaTimeModule.addDeserializer(LocalDate.class, new LocalDateDeserializer(new DateTimeFormatterBuilder() + .optionalStart() + .append(dateTimeFormatterAllowingSpace) + .optionalEnd() + .optionalStart() + .append(DateTimeFormatter.ofPattern("yyyyMMdd")) + .optionalEnd() + .toFormatter())); + return javaTimeModule; + } +} diff --git a/impl/src/main/java/slack/SlackModule.java b/impl/src/main/java/slack/SlackModule.java index 963f2cf..a73f755 100644 --- a/impl/src/main/java/slack/SlackModule.java +++ b/impl/src/main/java/slack/SlackModule.java @@ -28,5 +28,7 @@ public void configure(Binder binder) { slackMessageHandlerScanner.getMessageHandlers().forEach(aClass -> slackMessageHandlerMultibinder.addBinding().to(aClass)); binder.bind(SlackRTMClient.class).asEagerSingleton(); + + } } diff --git a/impl/src/main/resources/migrations.xml b/impl/src/main/resources/migrations.xml index 67e2275..84d1e90 100644 --- a/impl/src/main/resources/migrations.xml +++ b/impl/src/main/resources/migrations.xml @@ -202,4 +202,8 @@ + + + + diff --git a/impl/src/test/java/impl/QuestionResourceImplTest.java b/impl/src/test/java/impl/QuestionResourceImplTest.java index 3d7b7cc..249edd5 100644 --- a/impl/src/test/java/impl/QuestionResourceImplTest.java +++ b/impl/src/test/java/impl/QuestionResourceImplTest.java @@ -9,16 +9,21 @@ import org.assertj.core.api.ThrowableAssert; import org.junit.Before; import org.junit.Test; -import rx.Observable; +import se.fortnox.reactivewizard.CollectionOptions; import se.fortnox.reactivewizard.jaxrs.WebException; import slack.SlackConfig; import slack.SlackResource; import java.sql.SQLException; +import static impl.QuestionResourceImpl.FAILED_TO_ADD_QUESTION_TO_DATABASE; import static impl.QuestionResourceImpl.FAILED_TO_DELETE_QUESTION; +import static impl.QuestionResourceImpl.FAILED_TO_GET_LATEST_QUESTIONS; +import static impl.QuestionResourceImpl.FAILED_TO_GET_POPULAR_QUESTIONS; +import static impl.QuestionResourceImpl.FAILED_TO_GET_POPULAR_UNANSWERED_QUESTIONS; import static impl.QuestionResourceImpl.FAILED_TO_GET_QUESTIONS_FROM_DATABASE; import static impl.QuestionResourceImpl.FAILED_TO_GET_QUESTION_FROM_DATABASE; +import static impl.QuestionResourceImpl.FAILED_TO_GET_RECENTLY_ACCEPTED_QUESTIONS; import static impl.QuestionResourceImpl.FAILED_TO_UPDATE_QUESTION_TO_DATABASE; import static impl.QuestionResourceImpl.NOT_OWNER_OF_QUESTION; import static io.netty.handler.codec.http.HttpResponseStatus.FORBIDDEN; @@ -27,6 +32,7 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static rx.Observable.empty; @@ -41,6 +47,7 @@ public class QuestionResourceImplTest { private SlackResource slackResource; private Question question; private Auth auth; + private CollectionOptions options; @Before public void beforeEach() { @@ -53,14 +60,14 @@ public void beforeEach() { questionResource = new QuestionResourceImpl(questionDao, questionVoteDao, slackResource, new SlackConfig(), applicationConfig); auth = new Auth(123); question = createQuestion(123); - + options = new CollectionOptions(); } @Test public void shouldReturnInternalServerErrorWhenCreateQuestionFails() { // given db error Question question = new Question(); - when(questionDao.addQuestion(123, question)).thenReturn(Observable.error(new SQLException("poff"))); + when(questionDao.addQuestion(123, question)).thenReturn(error(new SQLException("poff"))); Auth auth = new Auth(); auth.setUserId(123); @@ -69,15 +76,15 @@ public void shouldReturnInternalServerErrorWhenCreateQuestionFails() { .isThrownBy(() -> questionResource.createQuestion(auth, question).toBlocking().singleOrDefault(null)) .satisfies(e -> { assertEquals(INTERNAL_SERVER_ERROR, e.getStatus()); - assertEquals("failed.to.add.question.to.database", e.getError()); + assertEquals(FAILED_TO_ADD_QUESTION_TO_DATABASE, e.getError()); }); } @Test public void shouldReturnInternalServerErrorWhenGetQuestionsFails() { - when(questionDao.getQuestions(123)).thenReturn(Observable.error(new SQLException("poff"))); + when(questionDao.getQuestions(123, options)).thenReturn(error(new SQLException("poff"))); - assertException(() -> questionResource.getQuestions(123).toBlocking().singleOrDefault(null), + assertException(() -> questionResource.getQuestions(123, options).toBlocking().singleOrDefault(null), INTERNAL_SERVER_ERROR, FAILED_TO_GET_QUESTIONS_FROM_DATABASE); } @@ -85,7 +92,7 @@ public void shouldReturnInternalServerErrorWhenGetQuestionsFails() { @Test public void shouldReturnInternalServerErrorWhenUpdateQuestionFails() { when(questionDao.getQuestion(123)).thenReturn(just(question)); - when(questionDao.updateQuestion(123, 123, question)).thenReturn(Observable.error(new SQLException("poff"))); + when(questionDao.updateQuestion(123, 123, question)).thenReturn(error(new SQLException("poff"))); assertException(() -> questionResource.updateQuestion(auth, 123, question).toBlocking().singleOrDefault(null), INTERNAL_SERVER_ERROR, @@ -106,7 +113,7 @@ public void shouldThrowInternalServerErrorIfQuestionCannotBeFetchedOnUpdate() { @Test public void shouldThrowInternalServerErrorIfQuestionCannotBeFetchedAfterUpdate() { - when(questionDao.getQuestion(123)).thenReturn(Observable.error(new SQLException("poff"))); + when(questionDao.getQuestion(123)).thenReturn(error(new SQLException("poff"))); assertException(() -> questionResource.updateQuestion(auth, 123, question).toBlocking().singleOrDefault(null), INTERNAL_SERVER_ERROR, @@ -123,10 +130,46 @@ public void shouldThrowForbiddenIfQuestionIsNotCreatedByTheDeleter() { NOT_OWNER_OF_QUESTION); } + @Test + public void shouldThrowInternalServerErrorIfLastestQuestionsCannotBeFetched() { + when(questionDao.getLatestQuestions(options)).thenReturn(error(new SQLException("poff"))); + + assertException(() -> questionResource.getLatestQuestions(options).toBlocking().singleOrDefault(null), + INTERNAL_SERVER_ERROR, + FAILED_TO_GET_LATEST_QUESTIONS); + } + + @Test + public void shouldThrowInternalServerErrorIfPopularQuestionsCannotBeFetched() { + when(questionDao.getPopularQuestions(any())).thenReturn(error(new SQLException("poff"))); + + assertException(() -> questionResource.getPopularQuestions(options).toBlocking().singleOrDefault(null), + INTERNAL_SERVER_ERROR, + FAILED_TO_GET_POPULAR_QUESTIONS); + } + + @Test + public void shouldThrowInternalServerErrorIfPopularUnansweredQuestionsCannotBeFetched() { + when(questionDao.getPopularUnansweredQuestions(options)).thenReturn(error(new SQLException("poff"))); + + assertException(() -> questionResource.getPopularUnansweredQuestions(options).toBlocking().singleOrDefault(null), + INTERNAL_SERVER_ERROR, + FAILED_TO_GET_POPULAR_UNANSWERED_QUESTIONS); + } + + @Test + public void shouldThrowInternalServerErrorIfRecentlyAcceptedQuestionsCannotBeFetched() { + when(questionDao.getRecentlyAcceptedQuestions(options)).thenReturn(error(new SQLException("poff"))); + + assertException(() -> questionResource.getRecentlyAcceptedQuestions(options).toBlocking().singleOrDefault(null), + INTERNAL_SERVER_ERROR, + FAILED_TO_GET_RECENTLY_ACCEPTED_QUESTIONS); + } + @Test public void shouldThrowInternalIfQuestionToDeleteCannotBeDeleted() { - when(questionDao.deleteQuestion(123, 123)).thenReturn(Observable.error(new SQLException("poff"))); + when(questionDao.deleteQuestion(123, 123)).thenReturn(error(new SQLException("poff"))); when(questionDao.getQuestion(123)).thenReturn(just(question)); assertException(() -> questionResource.deleteQuestion(auth, 123).toBlocking().singleOrDefault(null), diff --git a/spec/src/test/java/impl/AnswerResourceTest.java b/spec/src/test/java/impl/AnswerResourceTest.java index c0eb5fe..dc2e28b 100644 --- a/spec/src/test/java/impl/AnswerResourceTest.java +++ b/spec/src/test/java/impl/AnswerResourceTest.java @@ -515,7 +515,7 @@ private Question createQuestion(Auth auth) { questionResource.createQuestion(new MockAuth(auth.getUserId()), question).toBlocking().singleOrDefault(null); // then the question should be returned when asking for the users questions - List questions = questionResource.getQuestions(auth.getUserId()).toBlocking().single(); + List questions = questionResource.getQuestions(auth.getUserId(), null).toBlocking().single(); assertEquals(1, questions.size()); return questions.get(0); } diff --git a/spec/src/test/java/impl/QuestionResourceTest.java b/spec/src/test/java/impl/QuestionResourceTest.java index d7e2074..5a3d3bc 100644 --- a/spec/src/test/java/impl/QuestionResourceTest.java +++ b/spec/src/test/java/impl/QuestionResourceTest.java @@ -9,10 +9,12 @@ import api.auth.Auth; import com.github.seratch.jslack.api.model.block.SectionBlock; import com.github.seratch.jslack.api.model.block.composition.MarkdownTextObject; +import dao.AnswerDao; import dao.QuestionDao; import dao.QuestionVoteDao; import io.netty.handler.codec.http.HttpResponseStatus; import org.apache.log4j.Appender; +import org.assertj.core.internal.bytebuddy.utility.RandomString; import org.jetbrains.annotations.NotNull; import org.junit.After; import org.junit.Before; @@ -23,22 +25,32 @@ import org.testcontainers.containers.PostgreSQLContainer; import rx.Observable; import rx.observers.AssertableSubscriber; +import se.fortnox.reactivewizard.CollectionOptions; +import se.fortnox.reactivewizard.db.GeneratedKey; +import se.fortnox.reactivewizard.db.Update; import se.fortnox.reactivewizard.jaxrs.WebException; import se.fortnox.reactivewizard.test.LoggingMockUtil; import slack.SlackConfig; import slack.SlackResource; import java.sql.SQLException; +import java.time.LocalDateTime; import java.util.List; import java.util.function.BiFunction; +import java.util.function.Function; import static impl.QuestionResourceImpl.FAILED_TO_SEARCH_FOR_QUESTIONS; import static impl.QuestionResourceImpl.INVALID_VOTE; import static impl.QuestionResourceImpl.QUESTION_NOT_FOUND; +import static impl.TestSetup.getAnswer; import static impl.TestSetup.getQuestion; import static impl.TestSetup.insertUser; import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST; import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND; +import static java.time.LocalDateTime.now; +import static java.util.Arrays.asList; +import static java.util.Objects.nonNull; +import static java.util.stream.IntStream.range; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.junit.Assert.assertEquals; @@ -54,22 +66,33 @@ import static rx.Observable.empty; import static rx.Observable.error; import static se.fortnox.reactivewizard.test.TestUtil.matches; +import static util.ObservableAssertions.assertThat; +import static util.ObservableAssertions.assertThatList; public class QuestionResourceTest { + + public static final LocalDateTime CURRENT = now(); + public static final LocalDateTime A_DAY_AGO = CURRENT.minusDays(1); + public static final LocalDateTime A_MONTH_AGO = CURRENT.minusMonths(1); + private static QuestionResource questionResource; private static AnswerResource answerResource; private static UserResource userResource; private static QuestionVoteDao questionVoteDao; private static SlackResource slackResource; - private static QuestionDao questionDao; + private static QuestionDao questionDao; + private static AnswerDao answerDao; + private static TestDao testDao; @ClassRule public static PostgreSQLContainer postgreSQLContainer = new PostgreSQLContainer(); - private static TestSetup testSetup; - private static Appender appender; + private static TestSetup testSetup; + private static Appender appender; private static ApplicationConfig applicationConfig; + private CollectionOptions options; + @BeforeClass public static void before() { testSetup = new TestSetup(postgreSQLContainer); @@ -78,12 +101,19 @@ public static void before() { answerResource = testSetup.getInjector().getInstance(AnswerResource.class); questionVoteDao = testSetup.getInjector().getInstance(QuestionVoteDao.class); questionDao = testSetup.getInjector().getInstance(QuestionDao.class); + answerDao = testSetup.getInjector().getInstance(AnswerDao.class); + testDao = testSetup.getInjector().getInstance(TestDao.class); slackResource = mock(SlackResource.class); applicationConfig = new ApplicationConfig(); applicationConfig.setBaseUrl("deployed.fuel.com"); when(slackResource.postMessageToSlack(anyString(), any())).thenReturn(empty()); } + @Before + public void setupOptions() { + options = new CollectionOptions(); + } + @After public void afterEach() throws Exception { testSetup.clearDatabase(); @@ -104,7 +134,7 @@ public void shouldThrowErrorWhenServerIsDown() { when(questionDao.getLatestQuestions(any())).thenReturn(error(new SQLException())); try { - questionResource.getLatestQuestion(null).toBlocking().single(); + questionResource.getLatestQuestions(null).toBlocking().single(); fail("Should have thrown exception"); } catch (WebException e) { assertThat(e.getError()).isEqualTo("failed.to.get.latest.questions"); @@ -115,7 +145,7 @@ public void shouldThrowErrorWhenServerIsDown() { public void shouldBePossibleToGetQuestionBySlackThreadId() { TestSetup.insertUser(userResource); - Auth mockAuth = newAuth(); + Auth mockAuth = newAuth(); Question question = TestSetup.getQuestion("my question title", "my question"); String slackThreadId = String.valueOf(System.currentTimeMillis()); @@ -134,14 +164,32 @@ public void shouldReturnNotFoundWhenNoQuestionIsFoundForSlackThreadId() { test.awaitTerminalEvent(); test.assertError(WebException.class); - assertThat(((WebException)test.getOnErrorEvents().get(0)).getError()).isEqualTo("not.found"); + assertThat(((WebException)test.getOnErrorEvents().get(0)).getError()).isEqualTo(QUESTION_NOT_FOUND); + } + + @Test + public void shouldList10LatestQuestionsAsDefault() { + assertLimit(questionResource::getLatestQuestions, 10, 50); + } + + @Test + public void shouldList10PopularQuestionsAsDefault() { + assertLimit(questionResource::getPopularQuestions, 10, 50); } @Test - public void shouldListLatest10QuestionsAsDefault() { - generateQuestions(20); - List questions = questionResource.getLatestQuestion(null).toBlocking().single(); - assertEquals(10, questions.size()); + public void shouldList10PopularUnsansweredQuestionsAsDefault() { + assertLimit(questionResource::getPopularUnansweredQuestions, 10, 50); + } + + @Test + public void shouldList10RecentlyAcceptedQuestionsAsDefault() { + range(1, 15) + .forEach(i -> createQuestionWithAcceptedAnswer(CURRENT)); + + assertThatList(questionResource.getRecentlyAcceptedQuestions(options)) + .hasExactlyOne() + .hasSize(11); } @Test @@ -182,7 +230,7 @@ private Question createQuestionAndAnswer(Auth mockAuth) { } private Answer createAnswer(Auth mockAuth, long questionId, String answerBody) { - Answer answer = TestSetup.getAnswer(answerBody); + Answer answer = getAnswer(answerBody); answerResource.createAnswer(mockAuth, answer, questionId).toBlocking().singleOrDefault(null); return answer; } @@ -287,7 +335,7 @@ public void shouldBePossibleToAddQuestionAndFetchIt() { questionResource.createQuestion(mockAuth, question).toBlocking().singleOrDefault(null); // then the question should be returned when asking for the users questions - List questions = questionResource.getQuestions(createdUser.getId()).toBlocking().single(); + List questions = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single(); assertEquals(1, questions.size()); Question insertedQuestion = questions.get(0); @@ -306,7 +354,7 @@ public void shouldBePossibleToGetQuestionById() { Auth mockAuth = newAuth(); mockAuth.setUserId(mockAuth.getUserId()); questionResource.createQuestion(mockAuth, question).toBlocking().singleOrDefault(null); - List questions = questionResource.getQuestions(mockAuth.getUserId()).toBlocking().single(); + List questions = questionResource.getQuestions(mockAuth.getUserId(), null).toBlocking().single(); assertEquals(1, questions.size()); // then the question should be returned when asking for the specific question @@ -330,12 +378,12 @@ public void shouldBePossibleToUpdateQuestion() { mockAuth.setUserId(createdUser.getId()); questionResource.createQuestion(mockAuth, question).toBlocking().singleOrDefault(null); - Question storedQuestion = questionResource.getQuestions(createdUser.getId()).toBlocking().single().get(0); + Question storedQuestion = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single().get(0); storedQuestion.setBounty(400); storedQuestion.setTitle("new title"); storedQuestion.setQuestion("new question body"); questionResource.updateQuestion(mockAuth, storedQuestion.getId(), storedQuestion).toBlocking().singleOrDefault(null); - List questions = questionResource.getQuestions(createdUser.getId()).toBlocking().single(); + List questions = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single(); assertEquals(1, questions.size()); Question updatedQuestion = questions.get(0); @@ -363,7 +411,7 @@ public void shouldOnyReturnQuestionsForTheSpecifiedUser() { questionResource.createQuestion(authOtherUser, questionForOtherUser).toBlocking().singleOrDefault(null); // then only questions for our user should be returned - List questions = questionResource.getQuestions(ourUser.getId()).toBlocking().single(); + List questions = questionResource.getQuestions(ourUser.getId(), null).toBlocking().single(); assertEquals(1, questions.size()); Question insertedQuestion = questions.get(0); @@ -388,22 +436,22 @@ public void shouldBePossibleToDeleteQuestion() { questionResource.createQuestion(mockAuth, questionToInsert2).toBlocking().singleOrDefault(null); // and the questions has answers - List questions = questionResource.getQuestions(createdUser.getId()).toBlocking().single(); + List questions = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single(); Answer answer = new Answer(); answer.setAnswer("just a answer"); answerResource.createAnswer(mockAuth, answer, questions.get(0).getId()).toBlocking().singleOrDefault(null); answerResource.createAnswer(mockAuth, answer, questions.get(1).getId()).toBlocking().singleOrDefault(null); - List questionsSaved = questionResource.getQuestions(createdUser.getId()).toBlocking().single(); + List questionsSaved = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single(); // when we deletes a question questionResource.deleteQuestion(mockAuth, questionsSaved.get(0).getId()).toBlocking().singleOrDefault(null); // only the question we want to delete should be removed - List remaningQuestions = questionResource.getQuestions(createdUser.getId()).toBlocking().single(); + List remaningQuestions = questionResource.getQuestions(createdUser.getId(), null).toBlocking().single(); assertThat(remaningQuestions.size()).isEqualTo(1); - assertThat(remaningQuestions.get(0).getTitle()).isEqualTo("my question title2"); + assertThat(remaningQuestions.get(0).getTitle()).isEqualTo(questionsSaved.get(1).getTitle()); // and the answers should be deleted as well for the deleted question List answersForTheNonDeletedQuestion = answerResource.getAnswers(mockAuth, questionsSaved.get(1).getId()).toBlocking().singleOrDefault(null); @@ -430,7 +478,7 @@ public void shouldNotDeleteQuestionThatDoesNotExist() { @Test public void shouldLogThatWeCouldNotSendSlackNotificationWhenQuestionIsCreated() { // given bad slack config - Auth auth = new Auth(); + Auth auth = new Auth(); Question question = TestSetup.getQuestion("title", "body"); when(slackResource.postMessageToSlack(eq("rocket-fuel"), any())).thenReturn(error(new SQLException("poff"))); QuestionResource questionResource = new QuestionResourceImpl(questionDao, questionVoteDao, slackResource, new SlackConfig(), applicationConfig); @@ -448,7 +496,7 @@ public void shouldLogThatWeCouldNotSendSlackNotificationWhenQuestionIsCreated() @Test public void shouldSendCorrectMessageToSlack() { // given that we have a question that we want to save - Auth auth = new Auth(); + Auth auth = new Auth(); Question question = TestSetup.getQuestion("title of question?", "who does one do?"); when(slackResource.postMessageToSlack(eq("rocket-fuel"), any())).thenReturn(empty()); QuestionResource questionResource = new QuestionResourceImpl(questionDao, questionVoteDao, slackResource, new SlackConfig(), applicationConfig); @@ -459,10 +507,10 @@ public void shouldSendCorrectMessageToSlack() { // then a message shall be sent through slack that a new question has been submitted. verify(slackResource, times(1)).postMessageToSlack(any(), mapArgumentCaptor.capture()); - SectionBlock header = (SectionBlock) mapArgumentCaptor.getValue().get(0); - SectionBlock content = (SectionBlock) mapArgumentCaptor.getValue().get(1); + SectionBlock header = (SectionBlock)mapArgumentCaptor.getValue().get(0); + SectionBlock content = (SectionBlock)mapArgumentCaptor.getValue().get(1); - String headerText = ((MarkdownTextObject)header.getText()).getText(); + String headerText = ((MarkdownTextObject)header.getText()).getText(); String contentText = ((MarkdownTextObject)content.getText()).getText(); assertThat(headerText).isEqualTo("A new question: *title of question?* was submitted."); assertThat(contentText).isEqualTo("Head over to to view the question."); @@ -510,17 +558,140 @@ public void shouldThrowErrorWhenUserVotesOnOwnAnswer() { public void shouldListLatest5Questions() { int limit = 5; int questionsToGenerate = 10; + options.setLimit(limit); generateQuestions(questionsToGenerate); - List questions = questionResource.getLatestQuestion(limit).toBlocking().single(); - assertEquals(limit, questions.size()); + assertThatList(questionResource.getLatestQuestions(options)) + .hasExactlyOne() + .hasSize(limit); + } + + @Test + public void shouldSortLatestQuestions() { + List inExpectedOrder = asList( + createQuestionWithoutVotes(CURRENT), + createQuestionWithUnacceptedAnswer(CURRENT.minusMinutes(1), 3), + createQuestionWithoutVotes(A_DAY_AGO), + createQuestionWithoutAnswer(A_DAY_AGO.minusMinutes(1), 3), + createQuestionWithoutVotes(A_MONTH_AGO) + ); + assertOrder(questionResource::getLatestQuestions, inExpectedOrder); + } + + @Test + public void shouldSortPopularQuestions() { + List inExpectedOrder = asList( + createQuestionWithUnacceptedAnswer(CURRENT, 3), + createQuestionWithoutAnswer(CURRENT.minusMinutes(1), 3), + createQuestionWithUnacceptedAnswer(A_DAY_AGO, 3), + createQuestionWithUnacceptedAnswer(A_MONTH_AGO, 3), + createQuestionWithUnacceptedAnswer(CURRENT, 2), + createQuestionWithUnacceptedAnswer(CURRENT, 1), + createQuestionWithUnacceptedAnswer(CURRENT, 0) + ); + assertOrder(questionResource::getPopularQuestions, inExpectedOrder); + } + + @Test + public void shouldSortPopularUnansweredQuestions() { + List inExpectedOrder = asList( + createQuestionWithoutAnswer(CURRENT, 3), + createQuestionWithoutAnswer(A_DAY_AGO, 3), + createQuestionWithoutAnswer(A_MONTH_AGO, 3), + createQuestionWithoutAnswer(CURRENT, 2), + createQuestionWithoutAnswer(CURRENT, 1) + ); + + // red herrings + createQuestionWithUnacceptedAnswer(CURRENT, 3); + createQuestionWithAcceptedAnswer(CURRENT); + + assertOrder(questionResource::getPopularUnansweredQuestions, inExpectedOrder); + } + + @Test + public void shouldSortRecentlyAcceptedQuestions() { + List inExpectedOrder = asList( + createQuestionWithAcceptedAnswer(CURRENT), + createQuestionWithAcceptedAnswer(A_DAY_AGO), + createQuestionWithAcceptedAnswer(A_MONTH_AGO) + ); + + // red herrings + createQuestionWithUnacceptedAnswer(CURRENT, 3); + createQuestionWithoutAnswer(CURRENT, 3); - for (int i = 0; i < limit; i++) { - Question insertedQuestion = questions.get(i); - assertEquals("my question title " + (questionsToGenerate - i), insertedQuestion.getTitle()); - assertEquals("my question", insertedQuestion.getQuestion()); + assertOrder(questionResource::getRecentlyAcceptedQuestions, inExpectedOrder); + } + + @Test + public void shouldSortUsersQuestionsByCreated() { + Auth user = new MockAuth(insertUser(userResource).getId()); + long oldest = createQuestionForUser(user, A_MONTH_AGO); + long old = createQuestionForUser(user, A_DAY_AGO); + long current = createQuestionForUser(user, CURRENT); + + assertOrder(options -> questionResource.getQuestions(user.getUserId(), options), asList(current, old, oldest)); + } + + private void assertOrder(Function>> method, List inExpectedOrder) { + assertThatList(method.apply(options)) + .hasExactlyOne() + .extracting(Question::getId) + .containsExactlyElementsOf(inExpectedOrder); + } + + private Long createQuestionWithoutVotes(LocalDateTime created) { + return createQuestion("a", created, null, 0, true); + } + + private Long createQuestionWithUnacceptedAnswer(LocalDateTime created, int votes) { + return createQuestion("a", created, null, votes, true); + } + + private Long createQuestionWithoutAnswer(LocalDateTime created, int votes) { + return createQuestion("a", created, null, votes, false); + } + + private Long createQuestionWithAcceptedAnswer(LocalDateTime accepted) { + return createQuestion("a", now(), accepted, 0, true); + } + + private Long createQuestionForUser(Auth user, LocalDateTime created) { + return createQuestion(user, "title", created, null, 0, false); + } + + private Long createQuestion(String title, LocalDateTime created, LocalDateTime accepted, int votes, boolean createUnAcceptedAnswer) { + return createQuestion(new MockAuth(insertUser(userResource).getId()), title, created, accepted, votes, createUnAcceptedAnswer); + } + + private Long createQuestion(Auth user, String title, LocalDateTime created, LocalDateTime accepted, int votes, boolean createUnAcceptedAnswer) { + + Question question = questionDao.addQuestion(user.getUserId(), getQuestion(title, RandomString.make())) + .map(GeneratedKey::getKey) + .flatMap(questionDao::getQuestion) + .toBlocking().single(); + testDao.setCreatedAt(question, created).toBlocking().single(); + + range(0, votes) + .forEach(i -> assertThat(questionResource.upVoteQuestion(newAuth(), question.getId())).isEmpty()); + + assertThat(questionResource.getQuestion(user, question.getId())) + .hasExactlyOne() + .satisfies(q -> assertThat(q.getVotes()).isEqualTo(votes)); + + if(createUnAcceptedAnswer) { + assertThat(answerResource.createAnswer(user, getAnswer(RandomString.make()), question.getId())) + .hasExactlyOne(); } + + if (nonNull(accepted)) { + Answer acceptedAnswer = answerResource.createAnswer(user, getAnswer(RandomString.make()), question.getId()).toBlocking().firstOrDefault(null); + acceptedAnswer.setAcceptedAt(accepted); + assertThat(answerDao.updateAnswer(user.getUserId(), acceptedAnswer.getId(), acceptedAnswer)).isEmpty(); + } + return question.getId(); } private void voteAndAssertSuccess(BiFunction> call, Auth auth, Question question, int expectedVotes) { @@ -567,4 +738,25 @@ private void generateQuestions(int questionsToGenerate) { } } + private void assertLimit(Function>> method, int defaultLimit, int maxLimit) { + generateQuestions(maxLimit + 5); + + assertThatList(method.apply(options)) + .hasExactlyOne() + .describedAs("default limit") + .hasSize(defaultLimit + 1); // because CollectionOptionsQueryPart adds one to see if there are more + + options.setLimit(maxLimit + 5); + assertThatList(method.apply(options)) + .hasExactlyOne() + .describedAs("max limit") + .hasSize(maxLimit + 1); // because CollectionOptionsQueryPart adds one to see if there are more + } + + private interface TestDao { + @Update("UPDATE question " + + "SET created_at=:createdAt " + + "WHERE question.id=:question.id") + Observable setCreatedAt(Question question, LocalDateTime createdAt); + } } diff --git a/spec/src/test/java/util/ObservableListAssert.java b/spec/src/test/java/util/ObservableListAssert.java index c97633e..c9676d1 100644 --- a/spec/src/test/java/util/ObservableListAssert.java +++ b/spec/src/test/java/util/ObservableListAssert.java @@ -24,7 +24,7 @@ public ObservableListAssert(Observable> actual) { * @return a new assertion object whose object under test is the received List of E */ public ListAssert hasExactlyOne() { - List> values = actual.test().awaitTerminalEvent().getOnNextEvents(); + List> values = actual.test().awaitTerminalEvent().assertNoErrors().getOnNextEvents(); Assertions.assertThat(values) .hasSize(1); return Assertions.assertThat(values.get(0)); diff --git a/ui/src/components/questions/post.js b/ui/src/components/questions/post.js index a1ac32d..fe6acdd 100644 --- a/ui/src/components/questions/post.js +++ b/ui/src/components/questions/post.js @@ -69,12 +69,12 @@ class Post extends React.Component { ); } - getTime() { - return moment(this.props.created).fromNow(); + static getTime(time) { + return moment(time).fromNow(); } - getTimeStamp() { - return moment(this.props.created).format('llll'); + static getTimeStamp(time) { + return moment(time).format('llll'); } renderAnswered() { @@ -158,7 +158,7 @@ class Post extends React.Component { postingToServer: true, editFormError: '' }); - Answer.updateAnswer(this.props.answerId, {answer: this.state.answer}) + Answer.updateAnswer(this.props.answerId, { answer: this.state.answer }) .then(() => { this.setState({ isEditDialogOpen: false, @@ -215,7 +215,7 @@ class Post extends React.Component { renderVote(isVoteAllowed, onVote, faClass) { return (
- `${isVoteAllowed ? onVote(this.props.answerId || this.props.questionId) : ''}`} className={'fa ' + faClass}/> + `${isVoteAllowed ? onVote(this.props.answerId || this.props.questionId) : ''}`} className={'fa ' + faClass} />
) } @@ -296,11 +296,11 @@ class Post extends React.Component {
- {this.props.userName}/ + {this.props.userName}
{this.props.userName}
- {this.getTime()} + {Post.getTime(this.props.created)}
{this.renderButtons()} @@ -329,11 +329,15 @@ Post.defaultProps = { questionId: null, answerId: null, picture: null, - onDelete: () => {}, - onEdit: () => {}, + onDelete: () => { + }, + onEdit: () => { + }, enableAccept: false, - onUpVote: () => {}, - onDownVote: () => {}, + onUpVote: () => { + }, + onDownVote: () => { + }, allowUpVote: false, allowDownVote: false }; diff --git a/ui/src/components/questions/question.js b/ui/src/components/questions/question.js index a39ca96..05930c9 100644 --- a/ui/src/components/questions/question.js +++ b/ui/src/components/questions/question.js @@ -1,7 +1,6 @@ import React from 'react'; import Post from './post'; import { UserContext } from '../../usercontext'; -import Answer from './answer'; class Question extends React.Component { render() { diff --git a/ui/src/components/questions/questionrow.js b/ui/src/components/questions/questionrow.js index 8b21a56..a523135 100644 --- a/ui/src/components/questions/questionrow.js +++ b/ui/src/components/questions/questionrow.js @@ -1,16 +1,10 @@ import React from 'react'; import { withRouter } from 'react-router-dom'; -import moment from 'moment'; import Certificate from '../utils/certificate'; -import Coins from '../utils/coins'; import Trophy from '../utils/trophy'; import { UserContext } from '../../usercontext'; -import Dropdown from '../utils/dropdown'; -import { t } from 'ttag'; -import Dialog from '../utils/dialog'; -import Button from '../forms/button'; -import * as Question from '../../models/question'; +import Post from './post'; class QuestionRow extends React.Component { constructor(props) { @@ -28,10 +22,6 @@ class QuestionRow extends React.Component { }); } - getTime() { - return moment(this.props.question.createdAt).fromNow(); - } - getState() { if (this.props.question.answerAccepted) { return 'answered'; @@ -41,9 +31,6 @@ class QuestionRow extends React.Component { printCertificate() { const hasAccepted = this.props.question.answerAccepted; - if (!hasAccepted) { - return null; - } return ; } @@ -52,15 +39,15 @@ class QuestionRow extends React.Component { if (!hasEnoughVotes) { return null; } - return ; + return ; } getClasses() { - let classes = 'question-row flex'; - if (this.props.small) { - classes = `${classes} small`; - } - return classes; + return [ + 'question-row flex', + this.props.small ? 'small' : '', + this.props.question.answerAccepted ? 'accepted' : '', + ].join(' '); } renderTags() { @@ -95,7 +82,7 @@ class QuestionRow extends React.Component { {this.props.question.title}
- {this.props.question.createdBy}, {this.getTime()} + {Post.getTime(this.props.question.createdAt)}
diff --git a/ui/src/models/question.js b/ui/src/models/question.js index 373eb6a..983c7f6 100644 --- a/ui/src/models/question.js +++ b/ui/src/models/question.js @@ -1,14 +1,14 @@ import ApiFetch from '../components/utils/apifetch'; -export function getQuestionsFromUser(userId) { +export function getQuestionsFromUser(userId, limit = 10) { const options = { - url: `/api/users/${userId}/questions` + url: `/api/users/${userId}/questions?limit=${limit}` }; return ApiFetch(options); } -export function getLatestQuestion(limit = 10) { +export function getLatestQuestions(limit = 10) { const options = { url: `/api/questions/latest?limit=${limit}` }; @@ -16,6 +16,30 @@ export function getLatestQuestion(limit = 10) { return ApiFetch(options); } +export function getPopularQuestions(limit = 10) { + const options = { + url: `/api/questions/popular?limit=${limit}` + }; + + return ApiFetch(options); +} + +export function getPopularUnansweredQuestions(limit = 10) { + const options = { + url: `/api/questions/popularunanswered?limit=${limit}` + }; + + return ApiFetch(options); +} + +export function getRecentlyAcceptedQuestions(limit = 10) { + const options = { + url: `/api/questions/recentlyaccepted?limit=${limit}` + }; + + return ApiFetch(options); +} + export function getQuestionById(id) { const options = { url: `/api/questions/${id}` diff --git a/ui/src/style/components/_badges.scss b/ui/src/style/components/_badges.scss index f7f57d7..cfbd67e 100644 --- a/ui/src/style/components/_badges.scss +++ b/ui/src/style/components/_badges.scss @@ -1,5 +1,5 @@ .certificate, .trophy { - font-size: 1.2em; + font-size: 1em; position: relative; display: flex; justify-content: center; diff --git a/ui/src/style/layout/_grid.scss b/ui/src/style/layout/_grid.scss index 7faa92b..d80bdb5 100644 --- a/ui/src/style/layout/_grid.scss +++ b/ui/src/style/layout/_grid.scss @@ -3,8 +3,12 @@ flex-flow: wrap; &.spacing { - > [class^="col-"]:not(:first-child) { - padding-left: 15px; + > [class^="col-"] { + padding-bottom: 25px; + + &:not(:first-child) { + padding-left: 25px; + } } } } @@ -19,31 +23,50 @@ } } -@media screen and (max-width: $break-small) { +@media screen and (max-width: $break-large) { @for $i from 1 through 12 { + @if($i % 2 != 0) { + .col-#{$i} { + width: 100%; + } + .row.spacing { + .col-#{$i} { + padding-left: 0; + } + } + } + .col-#{$i} { - width: 100%; + width: 50%; + } + } + + .row.spacing { + [class^="col-"]:nth-child(odd) { + padding-left: 0; } } .hide-on-tablet { - display: inherit; + display: none; } } -@media screen and (min-width: $break-large) { + +@media screen and (max-width: $break-small) { @for $i from 1 through 12 { - @if($i % 2 != 0) { + .col-#{$i} { + width: 100%; + } + + .content .row.spacing { .col-#{$i} { - width: 100%; + padding-left: 0; } } - .col-#{$i} { - width: 50%; - } } - .hide-on-phone, .hide-on-tablet { + .hide-on-tablet, .hide-on-phone { display: none; } } diff --git a/ui/src/style/pages/_question.scss b/ui/src/style/pages/_question.scss index 26f0840..e5cde6e 100644 --- a/ui/src/style/pages/_question.scss +++ b/ui/src/style/pages/_question.scss @@ -83,8 +83,6 @@ $picture-size: 40px; &.answer { &.accepted .post-container { - // @include material-shadow(2); - // background-color: rgba(0, 0, 0, .05); background-color: rgba($notification-info, .18); } } diff --git a/ui/src/views/home.view.js b/ui/src/views/home.view.js index cd9352f..8b46b08 100644 --- a/ui/src/views/home.view.js +++ b/ui/src/views/home.view.js @@ -11,6 +11,9 @@ class HomeView extends React.Component { this.state = { userQuestions: [], latestQuestions: [], + popularQuestions: [], + popularUnansweredQuestions: [], + recentlyAcceptedQuestions: [], loaded: false }; } @@ -21,13 +24,19 @@ class HomeView extends React.Component { fetchQuestions() { const user = this.context.state.user; - const userQuestions = Question.getQuestionsFromUser(user.id); - const latestQuestions = Question.getLatestQuestion(); + const userQuestions = Question.getQuestionsFromUser(user.id,5); + const latestQuestions = Question.getLatestQuestions(5); + const popularQuestions = Question.getPopularQuestions(5); + const popularUnansweredQuestions = Question.getPopularUnansweredQuestions(5); + const recentlyAcceptedQuestions = Question.getRecentlyAcceptedQuestions(5); - Promise.all([userQuestions, latestQuestions]).then((response) => { + Promise.all([userQuestions, latestQuestions, popularQuestions, popularUnansweredQuestions, recentlyAcceptedQuestions]).then((response) => { this.setState({ userQuestions: response[0], latestQuestions: response[1], + popularQuestions: response[2], + popularUnansweredQuestions: response[3], + recentlyAcceptedQuestions: response[4], loaded: true }); }).catch(() => { @@ -49,6 +58,24 @@ class HomeView extends React.Component { }); } + getPopularQuestions() { + return this.state.popularQuestions.map((question, index) => { + return ; + }); + } + + getPopularUnansweredQuestions() { + return this.state.popularUnansweredQuestions.map((question, index) => { + return ; + }); + } + + getRecentlyAcceptedQuestions() { + return this.state.recentlyAcceptedQuestions.map((question, index) => { + return ; + }); + } + render() { if (!this.state.loaded) { return ( @@ -56,14 +83,32 @@ class HomeView extends React.Component { ); } return ( -
-
-
{t`Latest questions`}
- {this.getLatestQuestions()} +
+
+
+
{t`Latest questions`}
+ {this.getLatestQuestions()} +
+
+
{t`Popular questions`}
+ {this.getPopularQuestions()} +
+
+
+
+
{t`Popular unanswered questions`}
+ {this.getPopularUnansweredQuestions()} +
+
+
{t`Recently accepted questions`}
+ {this.getRecentlyAcceptedQuestions()} +
-
-
{t`Your recent questions`}
- {this.getUserQuestions()} +
+
+
{t`Your recent questions`}
+ {this.getUserQuestions()} +
) diff --git a/ui/src/views/question/question.view.js b/ui/src/views/question/question.view.js index 1308a33..188f228 100644 --- a/ui/src/views/question/question.view.js +++ b/ui/src/views/question/question.view.js @@ -3,15 +3,14 @@ import { t } from 'ttag'; import { withRouter } from 'react-router-dom'; import FillPage from '../../components/helpers/fillpage'; import Loader from '../../components/utils/loader'; -import Markdown from '../../components/helpers/markdown'; import InputField from '../../components/forms/inputfield'; import Button from '../../components/forms/button'; -import Post from '../../components/questions/post'; import Answer from '../../components/questions/answer'; import * as QuestionApi from '../../models/question'; import * as AnswerApi from '../../models/answer'; import { UserContext } from '../../usercontext'; import Question from '../../components/questions/question'; +import Post from '../../components/questions/post'; class QuestionView extends React.Component {