diff options
author | Jesse Morgan <jesse@jesterpm.net> | 2017-10-15 19:00:56 -0700 |
---|---|---|
committer | Jesse Morgan <jesse@jesterpm.net> | 2017-10-15 19:00:56 -0700 |
commit | b14ec9a9282cb49951b790ce1b48b1a078616926 (patch) | |
tree | deada14d4a407de18812d0e72c32136a2de2caa5 | |
parent | 79b8aacbb7b347bba9d14b1332666e7263a3a058 (diff) |
Refactor Chapter Ordering Logic20171015
The bug impacting the CCB integration was due to the "Introduction"
chapter having a higher "score" than every other chapter. It was a
mistake to use Score to compared chapter progress, particularly since
there are more chapters than scores.
This change gathers the chapter ordering logic, which was scattered
throughout the code into a new Chapters enum. Playlist and Chapter now
use Chapters as a key, instead of loose strings. Same for the
ProgressReporter interface.
17 files changed, 201 insertions, 199 deletions
diff --git a/src/main/java/com/p4square/f1oauth/F1ProgressReporter.java b/src/main/java/com/p4square/f1oauth/F1ProgressReporter.java index 8382020..1514043 100644 --- a/src/main/java/com/p4square/f1oauth/F1ProgressReporter.java +++ b/src/main/java/com/p4square/f1oauth/F1ProgressReporter.java @@ -1,6 +1,7 @@ package com.p4square.f1oauth; import com.p4square.grow.frontend.ProgressReporter; +import com.p4square.grow.model.Chapters; import org.apache.log4j.Logger; import org.restlet.security.User; @@ -29,8 +30,8 @@ public class F1ProgressReporter implements ProgressReporter { } @Override - public void reportChapterComplete(final User user, final String chapter, final Date date) { - final String attributeName = "Training Complete - " + chapter; + public void reportChapterComplete(final User user, final Chapters chapter, final Date date) { + final String attributeName = "Training Complete - " + chapter.toString().toLowerCase(); final Attribute attribute = new Attribute(attributeName); attribute.setStartDate(date); addAttribute(user, attribute); diff --git a/src/main/java/com/p4square/grow/backend/resources/TrainingRecordResource.java b/src/main/java/com/p4square/grow/backend/resources/TrainingRecordResource.java index 51ba56a..cec4328 100644 --- a/src/main/java/com/p4square/grow/backend/resources/TrainingRecordResource.java +++ b/src/main/java/com/p4square/grow/backend/resources/TrainingRecordResource.java @@ -6,13 +6,11 @@ package com.p4square.grow.backend.resources; import java.io.IOException; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.HashMap; +import java.util.*; import com.fasterxml.jackson.databind.ObjectMapper; +import com.p4square.grow.model.*; import org.restlet.data.MediaType; import org.restlet.data.Status; import org.restlet.resource.ServerResource; @@ -25,19 +23,12 @@ import org.apache.log4j.Logger; import com.p4square.grow.backend.GrowBackend; -import com.p4square.grow.model.Chapter; -import com.p4square.grow.model.Playlist; -import com.p4square.grow.model.VideoRecord; -import com.p4square.grow.model.TrainingRecord; - import com.p4square.grow.provider.CollectionProvider; import com.p4square.grow.provider.JsonEncodedProvider; import com.p4square.grow.provider.Provider; import com.p4square.grow.provider.ProvidesAssessments; import com.p4square.grow.provider.ProvidesTrainingRecords; -import com.p4square.grow.model.Score; - /** * * @author Jesse Morgan <jesse@jesterpm.net> @@ -199,7 +190,7 @@ public class TrainingRecordResource extends ServerResource { */ private void skipAssessedChapters(String userId, TrainingRecord record) { // Get the user's score. - Score assessedScore = new Score(0, 0); + final Score assessedScore; try { assessedScore = getAssessedScore(userId); @@ -211,19 +202,12 @@ public class TrainingRecordResource extends ServerResource { // Mark the correct videos as not required. Playlist playlist = record.getPlaylist(); - for (Map.Entry<String, Chapter> entry : playlist.getChaptersMap().entrySet()) { - String chapterId = entry.getKey(); + for (Map.Entry<Chapters, Chapter> entry : playlist.getChaptersMap().entrySet()) { + Chapters chapterId = entry.getKey(); Chapter chapter = entry.getValue(); - boolean required; - - if ("introduction".equals(chapter)) { - // Introduction chapter is always required - required = true; - - } else { - // Chapter required if the floor of the score is <= the chapter's numeric value. - required = assessedScore.floor() <= Score.numericScore(chapterId); - } + boolean required = chapterId.toScore() + .map(s -> assessedScore.floor() <= s) + .orElse(true); if (!required) { for (VideoRecord video : chapter.getVideos().values()) { diff --git a/src/main/java/com/p4square/grow/ccb/CCBProgressReporter.java b/src/main/java/com/p4square/grow/ccb/CCBProgressReporter.java index c352fc7..a89035d 100644 --- a/src/main/java/com/p4square/grow/ccb/CCBProgressReporter.java +++ b/src/main/java/com/p4square/grow/ccb/CCBProgressReporter.java @@ -3,12 +3,11 @@ package com.p4square.grow.ccb; import com.p4square.ccbapi.CCBAPI; import com.p4square.ccbapi.model.*; import com.p4square.grow.frontend.ProgressReporter; -import com.p4square.grow.model.Score; +import com.p4square.grow.model.Chapters; import org.apache.log4j.Logger; import org.restlet.security.User; import java.io.IOException; -import java.time.LocalDate; import java.time.ZoneId; import java.util.Date; @@ -41,11 +40,11 @@ public class CCBProgressReporter implements ProgressReporter { } final CCBUser ccbuser = (CCBUser) user; - updateLevelAndDate(ccbuser, GROW_ASSESSMENT, level, date); + updateLevelAndDate(ccbuser, GROW_ASSESSMENT, level.toLowerCase(), date); } @Override - public void reportChapterComplete(final User user, final String chapter, final Date date) throws IOException { + public void reportChapterComplete(final User user, final Chapters chapter, final Date date) throws IOException { if (!(user instanceof CCBUser)) { throw new IllegalArgumentException("Expected CCBUser but got " + user.getClass().getCanonicalName()); } @@ -56,7 +55,8 @@ public class CCBProgressReporter implements ProgressReporter { .getCustomPulldownFields().getByLabel(GROW_LEVEL); if (currentLevel != null) { - if (Score.numericScore(chapter) <= Score.numericScore(currentLevel.getSelection().getLabel())) { + Chapters currentChapter = Chapters.fromString(currentLevel.getSelection().getLabel()); + if (chapter.compareTo(currentChapter) <= 0) { LOG.debug("Not updating level for " + user.getIdentifier() + " because current level (" + currentLevel.getSelection().getLabel() + ") is greater than new level (" + chapter + ")"); @@ -64,7 +64,7 @@ public class CCBProgressReporter implements ProgressReporter { } } - updateLevelAndDate(ccbuser, GROW_LEVEL, chapter, date); + updateLevelAndDate(ccbuser, GROW_LEVEL, chapter.identifier(), date); } private void updateLevelAndDate(final CCBUser user, final String field, final String level, final Date date) diff --git a/src/main/java/com/p4square/grow/frontend/ChapterCompletePage.java b/src/main/java/com/p4square/grow/frontend/ChapterCompletePage.java index 7bda39c..b1926e8 100644 --- a/src/main/java/com/p4square/grow/frontend/ChapterCompletePage.java +++ b/src/main/java/com/p4square/grow/frontend/ChapterCompletePage.java @@ -5,10 +5,13 @@ package com.p4square.grow.frontend; import java.io.IOException; +import java.util.Arrays; import java.util.Date; import java.util.Map; +import java.util.Optional; import com.p4square.f1oauth.FellowshipOneIntegrationDriver; +import com.p4square.grow.model.Chapters; import freemarker.template.Template; import org.restlet.data.MediaType; @@ -49,7 +52,7 @@ public class ChapterCompletePage extends FreeMarkerPageResource { private Provider<String, TrainingRecord> mTrainingRecordProvider; private String mUserId; - private String mChapter; + private Chapters mChapter; @Override public void doInit() { @@ -60,7 +63,7 @@ public class ChapterCompletePage extends FreeMarkerPageResource { mJsonClient = new JsonRequestClient(getContext().getClientDispatcher()); mTrainingRecordProvider = new TrainingRecordProvider<String>( - new JsonRequestProvider<TrainingRecord>( + new JsonRequestProvider<>( getContext().getClientDispatcher(), TrainingRecord.class)) { @Override @@ -71,7 +74,7 @@ public class ChapterCompletePage extends FreeMarkerPageResource { mUserId = getRequest().getClientInfo().getUser().getIdentifier(); - mChapter = getAttribute("chapter"); + mChapter = Chapters.fromString(getAttribute("chapter")); } /** @@ -91,12 +94,12 @@ public class ChapterCompletePage extends FreeMarkerPageResource { } // Verify they completed the chapter. - Map<String, Boolean> chapters = trainingRecord.getPlaylist().getChapterStatuses(); + Map<Chapters, Boolean> chapters = trainingRecord.getPlaylist().getChapterStatuses(); Boolean completed = chapters.get(mChapter); if (completed == null || !completed) { // Redirect back to training page... String nextPage = mConfig.getString("dynamicRoot", ""); - nextPage += "/account/training/" + mChapter; + nextPage += "/account/training/" + mChapter.toString().toLowerCase(); getResponse().redirectSeeOther(nextPage); return new StringRepresentation("Redirecting to " + nextPage); } @@ -105,25 +108,18 @@ public class ChapterCompletePage extends FreeMarkerPageResource { assignAttribute(); // Find the next chapter - String nextChapter = null; - { - int min = Integer.MAX_VALUE; - for (Map.Entry<String, Boolean> chapter : chapters.entrySet()) { - int index = chapterIndex(chapter.getKey()); - if (!chapter.getValue() && index < min) { - min = index; - nextChapter = chapter.getKey(); - } - } - } + Optional<Chapters> nextChapter = Arrays.stream(Chapters.values()).filter(c -> !chapters.get(c)).findFirst(); String nextOverride = getQueryValue("next"); if (nextOverride != null) { - nextChapter = nextOverride; + nextChapter = Optional.of(Chapters.fromString(nextOverride)); } - root.put("stage", mChapter); - root.put("nextstage", nextChapter); + String nextChapterString = nextChapter.map(c -> c.toString().toLowerCase()).orElse(null); + + + root.put("stage", mChapter.toString().toLowerCase()); + root.put("nextstage", nextChapterString); /* * We will display one of two transitional pages: @@ -133,13 +129,13 @@ public class ChapterCompletePage extends FreeMarkerPageResource { * complete message. */ Template t = mGrowFrontend.getTemplate("templates/stage-" - + nextChapter + "-forward.ftl"); + + nextChapterString + "-forward.ftl"); if (t == null) { // Skip the chapter complete message for "Introduction" - if ("introduction".equals(mChapter)) { + if (mChapter == Chapters.INTRODUCTION) { String nextPage = mConfig.getString("dynamicRoot", ""); - nextPage += "/account/training/" + nextChapter; + nextPage += "/account/training/" + nextChapterString; getResponse().redirectSeeOther(nextPage); return new StringRepresentation("Redirecting to " + nextPage); } @@ -191,20 +187,4 @@ public class ChapterCompletePage extends FreeMarkerPageResource { return response; } - - int chapterIndex(String chapter) { - if ("leader".equals(chapter)) { - return 5; - } else if ("teacher".equals(chapter)) { - return 4; - } else if ("disciple".equals(chapter)) { - return 3; - } else if ("believer".equals(chapter)) { - return 2; - } else if ("seeker".equals(chapter)) { - return 1; - } else { - return Integer.MAX_VALUE; - } - } } diff --git a/src/main/java/com/p4square/grow/frontend/GroupLeaderTrainingPageResource.java b/src/main/java/com/p4square/grow/frontend/GroupLeaderTrainingPageResource.java deleted file mode 100644 index 3ab140e..0000000 --- a/src/main/java/com/p4square/grow/frontend/GroupLeaderTrainingPageResource.java +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2013 Jesse Morgan - */ - -package com.p4square.grow.frontend; - -/** - * Display the Group Leader training videos. - * - * @author Jesse Morgan <jesse@jesterpm.net> - */ -public class GroupLeaderTrainingPageResource extends TrainingPageResource { - private static final String[] CHAPTERS = { "leader" }; - - @Override - public void doInit() { - super.doInit(); - - mChapter = "leader"; - } - - @Override - public String[] getChaptersInOrder() { - return CHAPTERS; - } -} diff --git a/src/main/java/com/p4square/grow/frontend/ProgressReporter.java b/src/main/java/com/p4square/grow/frontend/ProgressReporter.java index 4662024..9f1e184 100644 --- a/src/main/java/com/p4square/grow/frontend/ProgressReporter.java +++ b/src/main/java/com/p4square/grow/frontend/ProgressReporter.java @@ -1,5 +1,6 @@ package com.p4square.grow.frontend; +import com.p4square.grow.model.Chapters; import org.restlet.security.User; import java.io.IOException; @@ -27,5 +28,5 @@ public interface ProgressReporter { * @param chapter The chapter completed. * @param date The completion date. */ - void reportChapterComplete(User user, String chapter, Date date) throws IOException; + void reportChapterComplete(User user, Chapters chapter, Date date) throws IOException; } diff --git a/src/main/java/com/p4square/grow/frontend/TrainingPageResource.java b/src/main/java/com/p4square/grow/frontend/TrainingPageResource.java index 108c7a7..f3efca1 100644 --- a/src/main/java/com/p4square/grow/frontend/TrainingPageResource.java +++ b/src/main/java/com/p4square/grow/frontend/TrainingPageResource.java @@ -5,14 +5,10 @@ package com.p4square.grow.frontend; import java.io.IOException; -import java.util.Collections; -import java.util.Comparator; -import java.util.Date; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.concurrent.ExecutorService; +import com.p4square.grow.model.Chapters; import freemarker.template.Template; import org.restlet.data.MediaType; @@ -46,8 +42,6 @@ import org.restlet.security.User; */ public class TrainingPageResource extends FreeMarkerPageResource { private static final Logger LOG = Logger.getLogger(TrainingPageResource.class); - - private static final String[] CHAPTERS = { "introduction", "seeker", "believer", "disciple", "teacher", "leader" }; private static final Comparator<Map<String, Object>> VIDEO_COMPARATOR = (left, right) -> { String leftNumberStr = (String) left.get("number"); String rightNumberStr = (String) right.get("number"); @@ -72,7 +66,7 @@ public class TrainingPageResource extends FreeMarkerPageResource { private FeedData mFeedData; // Fields pertaining to this request. - protected String mChapter; + protected Chapters mChapter; protected String mUserId; @Override @@ -99,7 +93,12 @@ public class TrainingPageResource extends FreeMarkerPageResource { mFeedData = new FeedData(getContext(), mConfig); - mChapter = getAttribute("chapter"); + String chapterName = getAttribute("chapter"); + if (chapterName == null) { + mChapter = null; + } else { + mChapter = Chapters.fromString(chapterName); + } mUserId = getRequest().getClientInfo().getUser().getIdentifier(); } @@ -117,20 +116,21 @@ public class TrainingPageResource extends FreeMarkerPageResource { } Playlist playlist = trainingRecord.getPlaylist(); - Map<String, Boolean> chapters = playlist.getChapterStatuses(); - Map<String, Boolean> allowedChapters = new LinkedHashMap<String, Boolean>(); + Map<Chapters, Boolean> chapters = playlist.getChapterStatuses(); + Map<String, Boolean> allowedChapters = new LinkedHashMap<>(); // The user is not allowed to view chapters after his highest completed chapter. // In this loop we find which chapters are allowed and check if the user tried // to skip ahead. boolean allowUserToSkip = mConfig.getBoolean("allowUserToSkip", false) || getQueryValue("magicskip") != null; - String defaultChapter = null; - String highestCompletedChapter = null; + Chapters defaultChapter = null; + Chapters highestCompletedChapter = null; boolean userTriedToSkip = false; int overallProgress = 0; boolean foundRequired = false; - for (String chapterId : getChaptersInOrder()) { + + for (Chapters chapterId : Chapters.values()) { boolean allowed = true; Boolean completed = chapters.get(chapterId); @@ -145,12 +145,12 @@ public class TrainingPageResource extends FreeMarkerPageResource { } else { allowed = allowUserToSkip; - if (!allowUserToSkip && chapterId.equals(mChapter)) { + if (!allowUserToSkip && chapterId == mChapter) { userTriedToSkip = true; } } - allowedChapters.put(chapterId, allowed); + allowedChapters.put(chapterId.identifier(), allowed); if (completed) { highestCompletedChapter = chapterId; @@ -160,18 +160,18 @@ public class TrainingPageResource extends FreeMarkerPageResource { } // Overall progress is the percentage of chapters complete - overallProgress = (int) ((double) overallProgress / getChaptersInOrder().length * 100); + overallProgress = (int) ((double) overallProgress / Chapters.values().length * 100); if (defaultChapter == null) { // Everything is completed... send them back to introduction. - defaultChapter = "introduction"; + defaultChapter = Chapters.INTRODUCTION; } if (mChapter == null || userTriedToSkip) { // No chapter was specified or the user tried to skip ahead. // Either case, redirect. String nextPage = mConfig.getString("dynamicRoot", ""); - nextPage += "/account/training/" + defaultChapter; + nextPage += "/account/training/" + defaultChapter.toString().toLowerCase(); getResponse().redirectSeeOther(nextPage); return new StringRepresentation("Redirecting to " + nextPage); } @@ -180,7 +180,7 @@ public class TrainingPageResource extends FreeMarkerPageResource { // Get videos for the chapter. List<Map<String, Object>> videos = null; { - JsonResponse response = backendGet("/training/" + mChapter); + JsonResponse response = backendGet("/training/" + mChapter.toString().toLowerCase()); if (!response.getStatus().isSuccess()) { setStatus(Status.CLIENT_ERROR_NOT_FOUND); return null; @@ -208,7 +208,7 @@ public class TrainingPageResource extends FreeMarkerPageResource { chapterProgress = chapterProgress * 100 / videos.size(); Map root = getRootObject(); - root.put("chapter", mChapter); + root.put("chapter", mChapter.identifier()); root.put("chapters", allowedChapters.keySet()); root.put("isChapterAllowed", allowedChapters); root.put("chapterProgress", chapterProgress); @@ -220,7 +220,7 @@ public class TrainingPageResource extends FreeMarkerPageResource { boolean showfeed = true; // Don't show the feed if the topic isn't allowed. - if (!FeedData.TOPICS.contains(mChapter)) { + if (!FeedData.TOPICS.contains(mChapter.identifier())) { showfeed = false; } @@ -236,7 +236,7 @@ public class TrainingPageResource extends FreeMarkerPageResource { final User user = getRequest().getClientInfo().getUser(); // Get the date of the highest completed chapter. final Date completionDate = playlist.getChaptersMap().get(highestCompletedChapter).getCompletionDate(); - final String completedChapter = highestCompletedChapter; + final Chapters completedChapter = highestCompletedChapter; mThreadPool.execute(() -> { try { mProgressReporter.reportChapterComplete(user, completedChapter, completionDate); @@ -260,13 +260,6 @@ public class TrainingPageResource extends FreeMarkerPageResource { } /** - * This method returns a list of chapters in the correct order. - */ - protected String[] getChaptersInOrder() { - return CHAPTERS; - } - - /** * @return The backend endpoint URI */ private String getBackendEndpoint() { diff --git a/src/main/java/com/p4square/grow/model/Chapter.java b/src/main/java/com/p4square/grow/model/Chapter.java index ac27de6..3306bf0 100644 --- a/src/main/java/com/p4square/grow/model/Chapter.java +++ b/src/main/java/com/p4square/grow/model/Chapter.java @@ -18,12 +18,12 @@ import com.fasterxml.jackson.annotation.JsonIgnore; * @author Jesse Morgan <jesse@jesterpm.net> */ public class Chapter implements Cloneable { - private String mName; + private Chapters mName; private Map<String, VideoRecord> mVideos; - public Chapter(String name) { + public Chapter(Chapters name) { mName = name; - mVideos = new HashMap<String, VideoRecord>(); + mVideos = new HashMap<>(); } /** @@ -36,7 +36,7 @@ public class Chapter implements Cloneable { /** * @return The Chapter name. */ - public String getName() { + public Chapters getName() { return mName; } @@ -45,7 +45,7 @@ public class Chapter implements Cloneable { * * @param name The name of the chapter. */ - public void setName(final String name) { + public void setName(final Chapters name) { mName = name; } diff --git a/src/main/java/com/p4square/grow/model/Chapters.java b/src/main/java/com/p4square/grow/model/Chapters.java new file mode 100644 index 0000000..175b6fb --- /dev/null +++ b/src/main/java/com/p4square/grow/model/Chapters.java @@ -0,0 +1,46 @@ +package com.p4square.grow.model; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonValue; + +import java.util.Optional; + +/** + * The chapters of the training section. + */ +public enum Chapters { + INTRODUCTION, + SEEKER, + BELIEVER, + DISCIPLE, + TEACHER, + LEADER; + + /** + * A case-insensitive version of Chapters.valueOf(). + */ + @JsonCreator + public static Chapters fromString(String s) { + return valueOf(s.toUpperCase()); + } + + @JsonValue + public String identifier() { + return toString().toLowerCase(); + } + + /** + * Convert the Chapter to a score, if possible. + */ + public Optional<Double> toScore() { + switch (this) { + case SEEKER: + case BELIEVER: + case DISCIPLE: + case TEACHER: + return Optional.of(Score.numericScore(this.toString())); + default: + return Optional.empty(); + } + } +} diff --git a/src/main/java/com/p4square/grow/model/Playlist.java b/src/main/java/com/p4square/grow/model/Playlist.java index 3e77ada..201b3e2 100644 --- a/src/main/java/com/p4square/grow/model/Playlist.java +++ b/src/main/java/com/p4square/grow/model/Playlist.java @@ -21,7 +21,7 @@ public class Playlist { /** * Map of Chapter ID to map of Video ID to VideoRecord. */ - private Map<String, Chapter> mPlaylist; + private Map<Chapters, Chapter> mPlaylist; private Date mLastUpdated; @@ -29,7 +29,7 @@ public class Playlist { * Construct an empty playlist. */ public Playlist() { - mPlaylist = new HashMap<String, Chapter>(); + mPlaylist = new HashMap<>(); mLastUpdated = new Date(0); // Default to a prehistoric date if we don't have one. } @@ -82,7 +82,7 @@ public class Playlist { /** * Add a video to the playlist. */ - public VideoRecord add(String chapterId, String videoId) { + public VideoRecord add(Chapters chapterId, String videoId) { Chapter chapter = mPlaylist.get(chapterId); if (chapter == null) { @@ -100,17 +100,24 @@ public class Playlist { * @param chapterId The name of the chapter. * @param chapter The Chapter object to add. */ - @JsonAnySetter - public void addChapter(String chapterId, Chapter chapter) { + public void addChapter(Chapters chapterId, Chapter chapter) { chapter.setName(chapterId); mPlaylist.put(chapterId, chapter); } /** + * Variation of addChapter() with a String key for Jackson. + */ + @JsonAnySetter + private void addChapter(String chapterName, Chapter chapter) { + addChapter(Chapters.fromString(chapterName), chapter); + } + + /** * @return a map of chapter id to chapter. */ @JsonAnyGetter - public Map<String, Chapter> getChaptersMap() { + public Map<Chapters, Chapter> getChaptersMap() { return mPlaylist; } @@ -118,10 +125,10 @@ public class Playlist { * @return The last chapter to be completed. */ @JsonIgnore - public Map<String, Boolean> getChapterStatuses() { - Map<String, Boolean> completed = new HashMap<String, Boolean>(); + public Map<Chapters, Boolean> getChapterStatuses() { + Map<Chapters, Boolean> completed = new HashMap<>(); - for (Map.Entry<String, Chapter> entry : mPlaylist.entrySet()) { + for (Map.Entry<Chapters, Chapter> entry : mPlaylist.entrySet()) { completed.put(entry.getKey(), entry.getValue().isComplete()); } @@ -131,7 +138,7 @@ public class Playlist { /** * @return true if all required videos in the chapter have been watched. */ - public boolean isChapterComplete(String chapterId) { + public boolean isChapterComplete(Chapters chapterId) { Chapter chapter = mPlaylist.get(chapterId); if (chapter != null) { return chapter.isComplete(); @@ -152,8 +159,8 @@ public class Playlist { return; } - for (Map.Entry<String, Chapter> entry : source.getChaptersMap().entrySet()) { - String chapterName = entry.getKey(); + for (Map.Entry<Chapters, Chapter> entry : source.getChaptersMap().entrySet()) { + Chapters chapterName = entry.getKey(); Chapter theirChapter = entry.getValue(); Chapter myChapter = mPlaylist.get(entry.getKey()); diff --git a/src/main/java/com/p4square/grow/model/Score.java b/src/main/java/com/p4square/grow/model/Score.java index 031c309..c9bda83 100644 --- a/src/main/java/com/p4square/grow/model/Score.java +++ b/src/main/java/com/p4square/grow/model/Score.java @@ -15,6 +15,8 @@ public class Score { * * This method satisfies the invariant for Score x: * numericScore(x.toString()) <= x.getScore() + * + * @throws IllegalArgumentException if the string is not a score name. */ public static double numericScore(String score) { score = score.toLowerCase(); @@ -28,7 +30,7 @@ public class Score { } else if ("seeker".equals(score)) { return 0; } else { - return Integer.MAX_VALUE; + throw new IllegalArgumentException("Invalid score " + score); } } diff --git a/src/main/java/com/p4square/grow/tools/AttributeBackfillTool.java b/src/main/java/com/p4square/grow/tools/AttributeBackfillTool.java index d7fd2ff..bf9ee7f 100644 --- a/src/main/java/com/p4square/grow/tools/AttributeBackfillTool.java +++ b/src/main/java/com/p4square/grow/tools/AttributeBackfillTool.java @@ -9,6 +9,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import com.p4square.grow.model.*; import org.restlet.Client; import org.restlet.Context; import org.restlet.data.Protocol; @@ -24,10 +25,6 @@ import com.p4square.grow.backend.dynamo.DynamoKey; import com.p4square.grow.config.Config; -import com.p4square.grow.model.Chapter; -import com.p4square.grow.model.Playlist; -import com.p4square.grow.model.TrainingRecord; -import com.p4square.grow.model.VideoRecord; import com.p4square.grow.provider.JsonEncodedProvider; /** @@ -221,7 +218,7 @@ public class AttributeBackfillTool { Playlist playlist = record.getPlaylist(); chapters: - for (Map.Entry<String, Chapter> entry : playlist.getChaptersMap().entrySet()) { + for (Map.Entry<Chapters, Chapter> entry : playlist.getChaptersMap().entrySet()) { Chapter chapter = entry.getValue(); // Find completion date @@ -237,7 +234,7 @@ chapters: } } - String attributeName = "Training Complete - " + entry.getKey(); + String attributeName = "Training Complete - " + entry.getKey().toString().toLowerCase(); // Check if the user already has the attribute. List<Attribute> attributes = f1.getAttribute(userId, attributeName); diff --git a/src/test/java/com/p4square/grow/backend/resources/TrainingRecordResourceTest.java b/src/test/java/com/p4square/grow/backend/resources/TrainingRecordResourceTest.java index db85051..8a052ef 100644 --- a/src/test/java/com/p4square/grow/backend/resources/TrainingRecordResourceTest.java +++ b/src/test/java/com/p4square/grow/backend/resources/TrainingRecordResourceTest.java @@ -7,6 +7,7 @@ package com.p4square.grow.backend.resources; import java.util.Map; import java.util.HashMap; +import com.p4square.grow.model.Chapters; import org.restlet.data.Method; import org.restlet.Request; import org.restlet.Response; @@ -46,13 +47,13 @@ public class TrainingRecordResourceTest extends ResourceTestBase { mResponse = new Response(mRequest); Playlist playlist = new Playlist(); - playlist.add("introduction", "intro-1"); - playlist.add("seeker", "seeker-1"); - playlist.add("believer", "believer-1"); - playlist.add("believer", "believer-2"); - playlist.add("disciple", "disciple-1"); - playlist.add("teacher", "teacher-1"); - playlist.add("leader", "leader-1"); + playlist.add(Chapters.INTRODUCTION, "intro-1"); + playlist.add(Chapters.SEEKER, "seeker-1"); + playlist.add(Chapters.BELIEVER, "believer-1"); + playlist.add(Chapters.BELIEVER, "believer-2"); + playlist.add(Chapters.DISCIPLE, "disciple-1"); + playlist.add(Chapters.TEACHER, "teacher-1"); + playlist.add(Chapters.LEADER, "leader-1"); mApplication.setDefaultPlaylist(playlist); } diff --git a/src/test/java/com/p4square/grow/ccb/CCBProgressReporterTest.java b/src/test/java/com/p4square/grow/ccb/CCBProgressReporterTest.java index 63a973a..7a2688c 100644 --- a/src/test/java/com/p4square/grow/ccb/CCBProgressReporterTest.java +++ b/src/test/java/com/p4square/grow/ccb/CCBProgressReporterTest.java @@ -2,6 +2,7 @@ package com.p4square.grow.ccb; import com.p4square.ccbapi.CCBAPI; import com.p4square.ccbapi.model.*; +import com.p4square.grow.model.Chapters; import org.easymock.Capture; import org.easymock.EasyMock; import org.junit.Before; @@ -65,7 +66,7 @@ public class CCBProgressReporterTest { .andReturn(growLevelDate).anyTimes(); EasyMock.expect(cache.getIndividualPulldownByLabel(GROW_LEVEL)) .andReturn(growLevelPulldown).anyTimes(); - EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_1, "Believer")) + EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_1, "believer")) .andReturn(believer).anyTimes(); // Setup the Grow Assessment field. @@ -81,7 +82,7 @@ public class CCBProgressReporterTest { .andReturn(growAssessmentDate).anyTimes(); EasyMock.expect(cache.getIndividualPulldownByLabel(ASSESSMENT_LEVEL)) .andReturn(growAssessmentPulldown).anyTimes(); - EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_2, "Believer")) + EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_2, "believer")) .andReturn(believer).anyTimes(); } @@ -115,7 +116,7 @@ public class CCBProgressReporterTest { replay(); // Test reporter - reporter.reportChapterComplete(user, "Believer", date); + reporter.reportChapterComplete(user, Chapters.BELIEVER, date); // Assert that the profile was updated. verify(); @@ -138,7 +139,7 @@ public class CCBProgressReporterTest { replay(); // Test reporter - reporter.reportChapterComplete(user, "Believer", date); + reporter.reportChapterComplete(user, Chapters.BELIEVER, date); // Assert that the profile was updated. verify(); @@ -157,7 +158,7 @@ public class CCBProgressReporterTest { replay(); // Test reporter - reporter.reportChapterComplete(user, "Believer", date); + reporter.reportChapterComplete(user, Chapters.BELIEVER, date); // Assert that the profile was updated. verify(); @@ -173,17 +174,21 @@ public class CCBProgressReporterTest { replay(); // Test reporter - reporter.reportChapterComplete(user, "Believer", date); + reporter.reportChapterComplete(user, Chapters.BELIEVER, date); // Assert that the profile was updated. verify(); } + /** + * Verify that the date is updated even if the level can't be found in the pulldown menu. + * @throws Exception + */ @Test public void testReportChapterCompleteNoSuchValue() throws Exception { // Setup mocks setupCacheMocks(); - EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_1, "Foo")) + EasyMock.expect(cache.getPulldownItemByName(LookupTableType.UDF_IND_PULLDOWN_1, "seeker")) .andReturn(null).anyTimes(); Capture<UpdateIndividualProfileRequest> reqCapture = EasyMock.newCapture(); EasyMock.expect(api.updateIndividualProfile(EasyMock.capture(reqCapture))) @@ -191,7 +196,7 @@ public class CCBProgressReporterTest { replay(); // Test reporter - reporter.reportChapterComplete(user, "Foo", date); + reporter.reportChapterComplete(user, Chapters.SEEKER, date); // Assert that the profile was updated. verify(); diff --git a/src/test/java/com/p4square/grow/model/PlaylistTest.java b/src/test/java/com/p4square/grow/model/PlaylistTest.java index 9c893f6..b6650ef 100644 --- a/src/test/java/com/p4square/grow/model/PlaylistTest.java +++ b/src/test/java/com/p4square/grow/model/PlaylistTest.java @@ -29,15 +29,15 @@ public class PlaylistTest { public void testPlaylistAndChapter() { // Create a playlist for the test Playlist playlist = new Playlist(); - playlist.add("chapter1", "video1"); - playlist.add("chapter1", "video2"); + playlist.add(Chapters.SEEKER, "video1"); + playlist.add(Chapters.SEEKER, "video2"); // Chapter should not be complete - assertFalse(playlist.isChapterComplete("chapter1")); + assertFalse(playlist.isChapterComplete(Chapters.SEEKER)); // We should find the chapter in the map - Map<String, Chapter> chapterMap = playlist.getChaptersMap(); - Chapter chapter1 = chapterMap.get("chapter1"); + Map<Chapters, Chapter> chapterMap = playlist.getChaptersMap(); + Chapter chapter1 = chapterMap.get(Chapters.SEEKER); assertTrue(null != chapter1); // We should find the videos in the map. @@ -53,8 +53,10 @@ public class PlaylistTest { video2.complete(); // Chapter should be complete now. - assertTrue(playlist.isChapterComplete("chapter1")); - assertFalse(playlist.isChapterComplete("bogusChapter")); + assertTrue(playlist.isChapterComplete(Chapters.SEEKER)); + + // But other chapters should not be complete. + assertFalse(playlist.isChapterComplete(Chapters.BELIEVER)); } /** @@ -75,15 +77,15 @@ public class PlaylistTest { @Test public void testMergePlaylist() { Playlist oldList = new Playlist(); - oldList.add("chapter1", "video1").setRequired(true); - oldList.add("chapter2", "video2").setRequired(false); - oldList.add("chapter2", "video3").complete(); + oldList.add(Chapters.SEEKER, "video1").setRequired(true); + oldList.add(Chapters.BELIEVER, "video2").setRequired(false); + oldList.add(Chapters.BELIEVER, "video3").complete(); oldList.setLastUpdated(new Date(100)); Playlist newList = new Playlist(); - newList.add("chapter1", "video4").setRequired(true); - newList.add("chapter2", "video5").setRequired(false); - newList.add("chapter3", "video6").setRequired(false); + newList.add(Chapters.SEEKER, "video4").setRequired(true); + newList.add(Chapters.BELIEVER, "video5").setRequired(false); + newList.add(Chapters.DISCIPLE, "video6").setRequired(false); newList.setLastUpdated(new Date(500)); // Verify that you can't merge the old into the new @@ -102,9 +104,9 @@ public class PlaylistTest { assertFalse(oldList.find("video6").getRequired()); // New Chapter added - Map<String, Chapter> chapters = oldList.getChaptersMap(); + Map<Chapters, Chapter> chapters = oldList.getChaptersMap(); assertEquals(3, chapters.size()); - assertTrue(null != chapters.get("chapter3")); + assertTrue(null != chapters.get(Chapters.DISCIPLE)); // Date updated assertEquals(newList.getLastUpdated(), oldList.getLastUpdated()); @@ -121,17 +123,17 @@ public class PlaylistTest { @Test public void testMergeMoveVideoRecord() { Playlist oldList = new Playlist(); - oldList.add("chapter1", "video1").setRequired(true); - VideoRecord toMove = oldList.add("chapter1", "video2"); + oldList.add(Chapters.SEEKER, "video1").setRequired(true); + VideoRecord toMove = oldList.add(Chapters.SEEKER, "video2"); toMove.setRequired(true); toMove.complete(); - oldList.add("chapter2", "video3").complete(); + oldList.add(Chapters.BELIEVER, "video3").complete(); oldList.setLastUpdated(new Date(100)); Playlist newList = new Playlist(); - newList.add("chapter1", "video1").setRequired(true); - newList.add("chapter2", "video2").setRequired(true); - newList.add("chapter3", "video3").complete(); + newList.add(Chapters.SEEKER, "video1").setRequired(true); + newList.add(Chapters.BELIEVER, "video2").setRequired(true); + newList.add(Chapters.DISCIPLE, "video3").complete(); newList.setLastUpdated(new Date(500)); // Merge the new list into the old and verify results @@ -143,12 +145,12 @@ public class PlaylistTest { assertTrue(oldList.find("video3").getComplete()); // toMove is in the correct chapter. - assertNull(oldList.getChaptersMap().get("chapter1").getVideoRecord("video2")); - VideoRecord afterMove = oldList.getChaptersMap().get("chapter2").getVideoRecord("video2"); + assertNull(oldList.getChaptersMap().get(Chapters.SEEKER).getVideoRecord("video2")); + VideoRecord afterMove = oldList.getChaptersMap().get(Chapters.BELIEVER).getVideoRecord("video2"); assertSame(toMove, afterMove); // video3 got moved to the new chapter3 - assertNull(oldList.getChaptersMap().get("chapter2").getVideoRecord("video3")); - assertTrue(oldList.getChaptersMap().get("chapter3").getVideoRecord("video3").getComplete()); + assertNull(oldList.getChaptersMap().get(Chapters.BELIEVER).getVideoRecord("video3")); + assertTrue(oldList.getChaptersMap().get(Chapters.DISCIPLE).getVideoRecord("video3").getComplete()); } } diff --git a/src/test/java/com/p4square/grow/model/ScoreTest.java b/src/test/java/com/p4square/grow/model/ScoreTest.java index 5c7be46..80066fa 100644 --- a/src/test/java/com/p4square/grow/model/ScoreTest.java +++ b/src/test/java/com/p4square/grow/model/ScoreTest.java @@ -64,6 +64,15 @@ public class ScoreTest { } /** + * Verify that numericScore() throws if a non-score is passed in. + */ + @Test(expected = IllegalArgumentException.class) + public void testInvalidScoreString() { + // Introduction is not a valid score. + Score.numericScore("introduction"); + } + + /** * Verify that toString() returns the correct mappings. */ @Test diff --git a/src/test/java/com/p4square/grow/model/TrainingRecordTest.java b/src/test/java/com/p4square/grow/model/TrainingRecordTest.java index 43caa90..a34befd 100644 --- a/src/test/java/com/p4square/grow/model/TrainingRecordTest.java +++ b/src/test/java/com/p4square/grow/model/TrainingRecordTest.java @@ -52,15 +52,15 @@ public class TrainingRecordTest { assertEquals(null, r); // isChapterComplete - assertTrue(playlist.isChapterComplete("seeker")); // Complete because not required. - assertTrue(playlist.isChapterComplete("disciple")); // Required and completed. - assertFalse(playlist.isChapterComplete("teacher")); // Not complete. + assertTrue(playlist.isChapterComplete(Chapters.SEEKER)); // Complete because not required. + assertTrue(playlist.isChapterComplete(Chapters.DISCIPLE)); // Required and completed. + assertFalse(playlist.isChapterComplete(Chapters.TEACHER)); // Not complete. // getChapterStatuses - Map<String, Boolean> statuses = playlist.getChapterStatuses(); - assertTrue(statuses.get("seeker")); // Complete because not required. - assertTrue(statuses.get("disciple")); // Required and completed. - assertFalse(statuses.get("teacher")); // Not complete. + Map<Chapters, Boolean> statuses = playlist.getChapterStatuses(); + assertTrue(statuses.get(Chapters.SEEKER)); // Complete because not required. + assertTrue(statuses.get(Chapters.DISCIPLE)); // Required and completed. + assertFalse(statuses.get(Chapters.TEACHER)); // Not complete. } /** |