Bug 4104: Added TODOs from code review and code formatting

java/com.sap.sailing.domain.common/src/com/sap/sailing/domain/common/dto/TagDTO.java

@@ -8,6 +8,9 @@ import com.sap.sse.common.TimePoint;
  * Used to send tags over network. Allows to create tags with all possible combinations of states (private/public and
  * valid/revoked).
  */
+// TODO: Remove max length
+// TODO: add Javadoc, which parameter has which meaning? what can be guessed from parameters (revokedAt = null)? what is
+// the key to identify a tag?
 public class TagDTO implements Serializable {
 
     private static final long serialVersionUID = 3907411584518452300L;

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/client/RaceTimesInfoProvider.java

@@ -23,6 +23,7 @@ import com.sap.sse.common.TimePoint;
 import com.sap.sse.gwt.client.ErrorReporter;
 import com.sap.sse.gwt.client.async.AsyncActionsExecutor;
 
+// TODO: add Javadoc
 public class RaceTimesInfoProvider {
     
     private final SailingServiceAsync sailingService;

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/RaceBoardPanel.java

@@ -305,11 +305,10 @@ public class RaceBoardPanel
         // add panel for tagging functionality, hidden if no url parameter "tags" is passed default
         taggingPanel = new TaggingPanel(parent, componentContext, stringMessages, sailingService, userService, timer, raceTimesInfoProvider);
         addChildComponent(taggingPanel);
-
-        if(Window.Location.getParameter("tag") != null) {
+        // TODO: Handle parameters as constructor parameters of TaggingPanel, URL params should be parsed in EntryPoint, not in Panel
+        if (Window.Location.getParameter("tag") != null) {
             taggingPanel.setVisible(true);
-        }
-        else {
+        } else {
             taggingPanel.setVisible(false);
         }
         

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/tagging/TagButtonJsonDeSerializer.java

@@ -13,6 +13,7 @@ import com.sap.sailing.gwt.ui.client.GwtJsonDeSerializer;
  * Serializes and deserializes {@link TagButton tag-buttons} to save them in the {@link com.sap.sse.security.UserStore
  * UserStore}.
  */
+// TODO: Don't use key for tag-buttons, save array directly instead.
 public class TagButtonJsonDeSerializer implements GwtJsonDeSerializer<List<TagButton>> {
 
     private static final String FIELD_TAG_BUTTONS = "tagButtons";

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/tagging/TagCell.java

@@ -24,6 +24,8 @@ import com.sap.sse.security.ui.client.UserService;
 /**
  * Used to display tags in various locations.
  */
+// TODO: Remove share button from private tags
+// TODO: change text of "created at" ...
 public class TagCell extends AbstractCell<TagDTO> {
 
     /**

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/tagging/TagInputPanel.java

@@ -13,6 +13,7 @@ import com.sap.sailing.gwt.ui.raceboard.tagging.TagPanelResources.TagPanelStyle;
 /**
  * Panel containing input fields for tag/tag button creation and modification.
  */
+// TODO: use DataEntryDialog
 public class TagInputPanel extends FlowPanel {
 
     private final TagPanelStyle style = TagPanelResources.INSTANCE.style();

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/tagging/TagSharedURLDialog.java

@@ -12,6 +12,10 @@ import com.sap.sailing.gwt.ui.raceboard.tagging.TagPanelResources.TagPanelStyle;
 /**
  * A Dialog to show the URL of shared tags
  */
+// TODO: adapt message instead of "URL:"
+// TODO: preselect link
+// TODO: add copy button for clipboard
+// TODO: texfield should not be editable
 public class TagSharedURLDialog extends DialogBox {
 
     private final TagPanelResources resources = TagPanelResources.INSTANCE;

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/raceboard/tagging/TaggingPanel.java

@@ -54,11 +54,17 @@ import com.sap.sse.security.ui.shared.UserDTO;
  * <br/>
  * The TaggingPanel is also used as a data provider for all of its subcomponents like header, footer and content
  * section. Therefore the TaggingPanel provides references to important services, string messages, its current state and
- * so on.
- * Best practice: The constructor of subcomponents of the TaggingPanel contains only the TaggingPanel as a parameter.
- * Every other required shared resource (string messages, service references, ...) can be requested from the
+ * so on. Best practice: The constructor of subcomponents of the TaggingPanel contains only the TaggingPanel as a
+ * parameter. Every other required shared resource (string messages, service references, ...) can be requested from the
  * TaggingPanel itself.
  */
+// TODO: Add refresh button which resets lastReceivedTagTime
+// TODO: resize plus of "add tags" button
+// TODO: get URL params as constructor parameter and not from Window object
+// TODO: Goal: Show tags in time slider (add bugzilla bug)
+// TODO: Unit Tests (incl. concatenation)
+// TODO: use HTML storage as event provider to update other tabs when user modifies private tags (new bugzilla bug)
+// TODO: cache user settings and use observer pattern for cache
 public class TaggingPanel extends ComponentWithoutSettings
         implements RaceTimesInfoProviderListener, UserStatusEventHandler, TimeListener {
 
@@ -103,8 +109,8 @@ public class TaggingPanel extends ComponentWithoutSettings
 
     // current state of the Tagging-Panel
     private State currentState;
-    
-    //Needed for sharing Tags 
+
+    // Needed for sharing Tags
     private boolean firstTimePublicTagsLoaded = true;
     private final TimePoint sharedTimePoint;
     private final String sharedTag;
@@ -140,25 +146,25 @@ public class TaggingPanel extends ComponentWithoutSettings
         raceTimesInfoProvider.addRaceTimesInfoProviderListener(this);
 
         setCurrentState(State.VIEW);
-        
-        //Get the url parameter "tag" and divide it into the logical timepoint and the tag title
-        String urlParameter = Window.Location.getParameter("tag");
-        if(urlParameter != null) {
-            if(urlParameter.length() > 13) {
-                String timeMillisString = urlParameter.substring(0, 13);//Works till "Nov 20 2286", afterwards timeMillis length increases from 13 to 14 chars
-                String tagString = urlParameter.substring(13, urlParameter.length()); 
+
+        // Get the url parameter "tag" and divide it into the logical timepoint and the tag title
+        final String urlParameter = Window.Location.getParameter("tag");
+        if (urlParameter != null) {
+            if (urlParameter.length() > 13) {
+                String timeMillisString = urlParameter.substring(0, 13);// Works till "Nov 20 2286", afterwards
+                                                                        // timeMillis length increases from 13 to 14
+                                                                        // chars
+                String tagString = urlParameter.substring(13, urlParameter.length());
                 sharedTimePoint = new MillisecondsTimePoint(Long.parseLong(timeMillisString));
-                sharedTag = tagString; 
-            }
-            else {
+                sharedTag = tagString;
+            } else {
                 Notification.notify(stringMessages.tagInvalidURL(), NotificationType.WARNING);
                 sharedTimePoint = null;
-                sharedTag = null; 
-            }  
-        }
-        else {
+                sharedTag = null;
+            }
+        } else {
             sharedTimePoint = null;
-            sharedTag = null; 
+            sharedTag = null;
         }
 
         initializePanel();
@@ -180,7 +186,6 @@ public class TaggingPanel extends ComponentWithoutSettings
         contentPanel.addStyleName(style.tagCellListPanel());
         contentPanel.add(tagCellList);
         contentPanel.add(createTagsButton);
-
         tagListProvider.addDataDisplay(tagCellList);
         tagCellList.setEmptyListWidget(new Label(stringMessages.tagNoTagsFound()));
         tagCellList.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.DISABLED);
@@ -197,7 +202,6 @@ public class TaggingPanel extends ComponentWithoutSettings
                 timer.addTimeListener(this);
             }
         });
-
         createTagsButton.setTitle(stringMessages.tagAddTags());
         createTagsButton.setStyleName(style.toggleEditState());
         createTagsButton.addStyleName(style.imagePusTransparent());
@@ -270,22 +274,18 @@ public class TaggingPanel extends ComponentWithoutSettings
             // tag does already exist
             Notification.notify(stringMessages.tagNotSavedReason(" " + stringMessages.tagAlreadyExists()),
                     NotificationType.WARNING);
-
         } else if (!isLoggedInAndRaceLogAvailable()) {
             // User is not logged in or race can not be identified because regatta, race column or fleet are missing.
             Notification.notify(stringMessages.tagNotSaved(), NotificationType.ERROR);
-
         } else if (tag.isEmpty()) {
             // Tag heading is empty. Empty tags are not allowed.
             Notification.notify(stringMessages.tagNotSpecified(), NotificationType.WARNING);
-
         } else {
             // replace null values with default values
             final String saveComment = (comment == null ? "" : comment);
             final String saveImageURL = (imageURL == null ? "" : imageURL);
             final TimePoint saveRaceTimePoint = (raceTimePoint == null ? new MillisecondsTimePoint(getTimerTime())
                     : raceTimePoint);
-
             sailingService.addTag(leaderboardName, raceColumn.getName(), fleet.getName(), tag, saveComment,
                     saveImageURL, visibleForPublic, saveRaceTimePoint, new AsyncCallback<SuccessInfo>() {
                         @Override
@@ -597,28 +597,28 @@ public class TaggingPanel extends ComponentWithoutSettings
                 if (modifiedTags) {
                     updateContent();
                 }
-                //After tags were added for the first time, find tag which matches the URL Parameter "tag", hightlight it and jump to its logical timepoint
-                if(firstTimePublicTagsLoaded && raceInfo.getTags() != null) {
+                // After tags were added for the first time, find tag which matches the URL Parameter "tag", hightlight
+                // it and jump to its logical timepoint
+                if (firstTimePublicTagsLoaded && raceInfo.getTags() != null) {
                     firstTimePublicTagsLoaded = false;
-                    if(sharedTimePoint != null) {
+                    if (sharedTimePoint != null) {
                         timer.setTime(sharedTimePoint.asMillis());
-                        if(sharedTag != null) {
+                        if (sharedTag != null) {
                             TagDTO matchingTag = null;
-                            for(TagDTO tag: tagListProvider.getAllTags()) {
-                                if(tag.getRaceTimepoint().equals(sharedTimePoint) && tag.getTag().equals(sharedTag)) {
+                            for (TagDTO tag : tagListProvider.getAllTags()) {
+                                if (tag.getRaceTimepoint().equals(sharedTimePoint) && tag.getTag().equals(sharedTag)) {
                                     matchingTag = tag;
                                     break;
                                 }
                             }
-                            if(matchingTag != null) {
+                            if (matchingTag != null) {
                                 tagSelectionModel.clear();
                                 tagSelectionModel.setSelected(matchingTag, true);
-                            }
-                            else {
+                            } else {
                                 Notification.notify(stringMessages.tagNotFound(), NotificationType.WARNING);
                             }
                         }
-                        
+
                     }
                 }
             });
@@ -649,12 +649,10 @@ public class TaggingPanel extends ComponentWithoutSettings
         raceTimesInfoProvider.getRaceIdentifiers().forEach((raceIdentifier) -> {
             raceTimesInfoProvider.setLatestReceivedTagTime(raceIdentifier, null);
         });
-
         // load content for new user
         reloadPrivateTags();
         filterbarPanel.loadTagFilterSets();
         footerPanel.loadAllTagButtons();
-
         // update UI
         setCurrentState(State.VIEW);
     }

java/com.sap.sailing.gwt.ui/src/main/java/com/sap/sailing/gwt/ui/server/SailingServiceImpl.java

@@ -2239,6 +2239,7 @@ public class SailingServiceImpl extends ProxiedRemoteServiceServlet
      * {@link RaceLogTagEvent tag events} since received timestamp (<code>latestReceivedTagTime</code>). Loads tags from
      * {@link ReadonlyRaceState cache} instead of scanning the whole {@link RaceLog} every request.
      */
+    // TODO: rename latestReceivedTagTime to match role
     @Override
     public RaceTimesInfoDTO getRaceTimesInfoIncludingTags(RegattaAndRaceIdentifier raceIdentifier,
             TimePoint latestReceivedTagTime) {

java/com.sap.sailing.server/src/com/sap/sailing/server/tagging/TaggingService.java

@@ -7,6 +7,15 @@ import com.sap.sailing.domain.common.RegattaAndRaceIdentifier;
 import com.sap.sailing.domain.common.dto.TagDTO;
 import com.sap.sse.common.TimePoint;
 
+// TODO: Add Javadoc, what does TaggingService do?
+// TODO: Replace error handling by throwing exceptions, see "topleveltranslations.json"
+// TODO: Remove public at interfaces
+// TODO: CommentTooLong
+// TODO: Remove max length
+// TODO: rename latestReceivedTagTime to match role
+// TODO: remove entry in settings if there are no private tags for this race anymore
+// TODO: rename keys to naming pattern (ssailing.tags....)
+// TODO: use document settings id for tags/tag-buttons/... as race identifier
 public interface TaggingService {
 
     /**

java/com.sap.sailing.server/src/com/sap/sailing/server/tagging/TaggingServiceImpl.java

@@ -104,7 +104,6 @@ public class TaggingServiceImpl implements TaggingService {
     private boolean addPrivateTag(String leaderboardName, String raceColumnName, String fleetName, String tag,
             String comment, String imageURL, TimePoint raceTimepoint) {
         boolean successful = true;
-
         SecurityService securityService = Activator.getSecurityService();
         if (securityService == null) {
             setLastErrorCode(ErrorCode.SECURITY_SERIVCE_NOT_FOUND);
@@ -136,7 +135,6 @@ public class TaggingServiceImpl implements TaggingService {
 
     private boolean removePublicTag(String leaderboardName, String raceColumnName, String fleetName, TagDTO tag) {
         boolean successful = true;
-
         RaceLog raceLog = racingService.getRaceLog(leaderboardName, raceColumnName, fleetName);
         if (raceLog == null) {
             setLastErrorCode(ErrorCode.RACELOG_NOT_FOUND);
@@ -183,7 +181,6 @@ public class TaggingServiceImpl implements TaggingService {
 
     private boolean removePrivateTag(String leaderboardName, String raceColumnName, String fleetName, TagDTO tag) {
         boolean successful = true;
-
         String username = getCurrentUsername();
         if (username == null) {
             setLastErrorCode(ErrorCode.NOT_LOGGED_IN);
@@ -198,7 +195,6 @@ public class TaggingServiceImpl implements TaggingService {
                 securityService.setPreference(username, key, serializer.serializeTags(privateTags));
             }
         }
-
         return successful;
     }
 
@@ -206,7 +202,6 @@ public class TaggingServiceImpl implements TaggingService {
     public boolean addTag(String leaderboardName, String raceColumnName, String fleetName, String tag, String comment,
             String imageURL, boolean visibleForPublic, TimePoint raceTimepoint) {
         boolean successful;
-
         // prefill optional parameters
         comment = comment == null ? "" : comment;
         imageURL = imageURL == null ? "" : imageURL;
@@ -241,7 +236,6 @@ public class TaggingServiceImpl implements TaggingService {
     @Override
     public boolean removeTag(String leaderboardName, String raceColumnName, String fleetName, TagDTO tag) {
         boolean successful = true;
-
         // check all parameters for validity
         if (tag == null) {
             setLastErrorCode(ErrorCode.TAG_NOT_EMPTY);
@@ -294,18 +288,12 @@ public class TaggingServiceImpl implements TaggingService {
     @Override
     public List<TagDTO> getPublicTags(RegattaAndRaceIdentifier raceIdentifier, TimePoint latestReceivedTagTime) {
         final List<TagDTO> result = new ArrayList<TagDTO>();
-
         TrackedRace trackedRace = racingService.getExistingTrackedRace(raceIdentifier);
         Iterable<RaceLog> raceLogs = trackedRace.getAttachedRaceLogs();
-
         for (RaceLog raceLog : raceLogs) {
             ReadonlyRaceState raceState = ReadonlyRaceStateImpl.getOrCreate(racingService, raceLog);
             Iterable<RaceLogTagEvent> foundTagEvents = raceState.getTagEvents();
             for (RaceLogTagEvent tagEvent : foundTagEvents) {
-                // TODO: As soon as permission-vertical branch got merged into master, apply
-                // new permission system at this if-statement and remove this old way of
-                // checking for permissions. (see bug 4104, comment 9)
-                // functionality: Check if user has the permission to see this tag.
                 if ((latestReceivedTagTime == null && tagEvent.getRevokedAt() == null)
                         || (latestReceivedTagTime != null && tagEvent.getRevokedAt() == null
                                 && tagEvent.getCreatedAt().after(latestReceivedTagTime))
@@ -317,14 +305,12 @@ public class TaggingServiceImpl implements TaggingService {
                 }
             }
         }
-
         return result;
     }
 
     @Override
     public List<TagDTO> getPrivateTags(String leaderboardName, String raceColumnName, String fleetName) {
         final List<TagDTO> result = new ArrayList<TagDTO>();
-
         SecurityService securityService = getSecurityService();
         if (securityService != null) {
             String username = getCurrentUsername();
@@ -335,7 +321,6 @@ public class TaggingServiceImpl implements TaggingService {
                 result.addAll(privateTags);
             }
         }
-
         return result;
     }