feat: users can add multiple instances of a series into a collection.

Fix #1123

Author: php-coder

NEWS.txt

@@ -1,4 +1,5 @@
 0.x (upcoming release)
+- (feature)        users can add multiple instances of a series into a collection
 - (infrastructure) all integration tests now pass on PostgreSQL
 - (infrastructure) migrate unit tests from Hamcrest to AssertJ
 

src/main/java/ru/mystamps/web/feature/collection/CollectionDao.java

@@ -21,6 +21,7 @@
 
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 @SuppressWarnings("PMD.TooManyMethods")
 public interface CollectionDao {
@@ -34,6 +35,7 @@ public interface CollectionDao {
 	Integer add(AddCollectionDbDto collection);
 	void markAsModified(Integer userId, Date updatedAt);
 	boolean isSeriesInUserCollection(Integer userId, Integer seriesId);
+	Map<Integer, Integer> findSeriesInstances(Integer userId, Integer seriesId);
 	Integer addSeriesToUserCollection(AddToCollectionDbDto dto);
 	void removeSeriesFromUserCollection(Integer userId, Integer seriesId);
 	CollectionInfoDto findCollectionInfoBySlug(String slug);

src/main/java/ru/mystamps/web/feature/collection/CollectionService.java

@@ -21,13 +21,15 @@
 
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 @SuppressWarnings("PMD.TooManyMethods")
 public interface CollectionService {
 	void createCollection(Integer ownerId, String ownerLogin);
 	void addToCollection(Integer userId, AddToCollectionDto dto);
-	void removeFromCollection(Integer userId, Integer seriesId);
+	void removeFromCollection(Integer userId, Integer seriesId, Integer seriesInstanceId);
 	boolean isSeriesInCollection(Integer userId, Integer seriesId);
+	Map<Integer, Integer> findSeriesInstances(Integer userId, Integer seriesId);
 	long countCollectionsOfUsers();
 	long countUpdatedSince(Date date);
 	long countSeriesOf(Integer collectionId);

src/main/java/ru/mystamps/web/feature/collection/CollectionServiceImpl.java

@@ -29,9 +29,14 @@
 
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 @RequiredArgsConstructor
-@SuppressWarnings("PMD.TooManyMethods")
+@SuppressWarnings({
+	"PMD.TooManyMethods",
+	// complains on "Series id must be non null"
+	"PMD.AvoidDuplicateLiterals"
+})
 public class CollectionServiceImpl implements CollectionService {
 
 	private final Logger log;
@@ -96,14 +101,22 @@ public void addToCollection(Integer userId, AddToCollectionDto dto) {
 	@Override
 	@Transactional
 	@PreAuthorize(HasAuthority.UPDATE_COLLECTION)
-	public void removeFromCollection(Integer userId, Integer seriesId) {
+	public void removeFromCollection(Integer userId, Integer seriesId, Integer seriesInstanceId) {
 		Validate.isTrue(userId != null, "User id must be non null");
 		Validate.isTrue(seriesId != null, "Series id must be non null");
+		Validate.isTrue(seriesInstanceId != null, "Series instance id must be non null");
 
-		collectionDao.removeSeriesFromUserCollection(userId, seriesId);
+		collectionDao.removeSeriesFromUserCollection(userId, seriesInstanceId);
 		collectionDao.markAsModified(userId, new Date());
 
-		log.info("Series #{} has been removed from collection", seriesId);
+		// The method accepts seriesId only for logging it.
+		// As seriesId is provided by user and we don't check whether it's related to
+		// seriesInstanceId, we can't fully rely on that but for the logging purposes it's enough
+		log.info(
+			"Series #{}: instance #{} has been removed from collection",
+			seriesId,
+			seriesInstanceId
+		);
 	}
 
 	@Override
@@ -119,6 +132,17 @@ public boolean isSeriesInCollection(Integer userId, Integer seriesId) {
 		return collectionDao.isSeriesInUserCollection(userId, seriesId);
 	}
 
+	// @todo #1123 CollectionService.findSeriesInstances(): add unit tests
+	@Override
+	@Transactional(readOnly = true)
+	@PreAuthorize(HasAuthority.UPDATE_COLLECTION)
+	public Map<Integer, Integer> findSeriesInstances(Integer userId, Integer seriesId) {
+		Validate.isTrue(userId != null, "User id must be non null");
+		Validate.isTrue(seriesId != null, "Series id must be non null");
+		
+		return collectionDao.findSeriesInstances(userId, seriesId);
+	}
+	
 	@Override
 	@Transactional(readOnly = true)
 	public long countCollectionsOfUsers() {

src/main/java/ru/mystamps/web/feature/collection/JdbcCollectionDao.java

@@ -23,12 +23,14 @@
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.dao.EmptyResultDataAccessException;
+import org.springframework.jdbc.core.ResultSetExtractor;
 import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
 import org.springframework.jdbc.support.GeneratedKeyHolder;
 import org.springframework.jdbc.support.KeyHolder;
 import ru.mystamps.web.common.JdbcUtils;
 import ru.mystamps.web.common.LinkEntityDto;
+import ru.mystamps.web.support.spring.jdbc.MapIntegerIntegerResultSetExtractor;
 
 import java.util.Collections;
 import java.util.Date;
@@ -41,6 +43,9 @@
 public class JdbcCollectionDao implements CollectionDao {
 	private static final Logger LOG = LoggerFactory.getLogger(JdbcCollectionDao.class);
 
+	private static final ResultSetExtractor<Map<Integer, Integer>> INSTANCES_COUNTER_EXTRACTOR =
+		new MapIntegerIntegerResultSetExtractor("id", "number_of_stamps");
+	
 	private final NamedParameterJdbcTemplate jdbcTemplate;
 
 	@Value("${collection.find_last_created}")
@@ -73,11 +78,14 @@ public class JdbcCollectionDao implements CollectionDao {
 	@Value("${collection.is_series_in_collection}")
 	private String isSeriesInUserCollectionSql;
 
+	@Value("${collection.find_series_instances}")
+	private String findSeriesInstancesSql;
+	
 	@Value("${collection.add_series_to_collection}")
 	private String addSeriesToCollectionSql;
 
-	@Value("${collection.remove_series_from_collection}")
-	private String removeSeriesFromCollectionSql;
+	@Value("${collection.remove_series_instance_from_collection}")
+	private String removeSeriesInstanceSql;
 
 	@Value("${collection.find_info_by_slug}")
 	private String findCollectionInfoBySlugSql;
@@ -218,6 +226,19 @@ public boolean isSeriesInUserCollection(Integer userId, Integer seriesId) {
 		return result > 0;
 	}
 
+	@Override
+	public Map<Integer, Integer> findSeriesInstances(Integer userId, Integer seriesId) {
+		Map<String, Object> params = new HashMap<>();
+		params.put("user_id", userId);
+		params.put("series_id", seriesId);
+		
+		return jdbcTemplate.query(
+			findSeriesInstancesSql,
+			params,
+			INSTANCES_COUNTER_EXTRACTOR
+		);
+	}
+	
 	@Override
 	public Integer addSeriesToUserCollection(AddToCollectionDbDto dto) {
 		Map<String, Object> params = new HashMap<>();
@@ -250,17 +271,17 @@ public Integer addSeriesToUserCollection(AddToCollectionDbDto dto) {
 
 	@SuppressWarnings("PMD.AvoidLiteralsInIfCondition")
 	@Override
-	public void removeSeriesFromUserCollection(Integer userId, Integer seriesId) {
+	public void removeSeriesFromUserCollection(Integer userId, Integer seriesInstanceId) {
 		Map<String, Object> params = new HashMap<>();
+		params.put("id", seriesInstanceId);
 		params.put("user_id", userId);
-		params.put("series_id", seriesId);
 
-		int affected = jdbcTemplate.update(removeSeriesFromCollectionSql, params);
+		int affected = jdbcTemplate.update(removeSeriesInstanceSql, params);
 		if (affected != 1) {
 			// CheckStyle: ignore LineLength for next 2 lines
 			LOG.warn(
-				"Unexpected number of affected rows after removing series #{} from collection of user #{}: {}",
-				seriesId,
+				"Unexpected number of affected rows after removing series instance #{} from collection of user #{}: {}",
+				seriesInstanceId,
 				userId,
 				affected
 			);

src/main/java/ru/mystamps/web/feature/series/SeriesController.java

@@ -412,6 +412,7 @@ public String addToCollection(
 
 		redirectAttributes.addFlashAttribute("justAddedSeries", true);
 		redirectAttributes.addFlashAttribute("justAddedSeriesId", seriesId);
+		redirectAttributes.addFlashAttribute("justAddedNumberOfStamps", form.getNumberOfStamps());
 
 		String collectionSlug = currentUserDetails.getUserCollectionSlug();
 		return redirectTo(CollectionUrl.INFO_COLLECTION_PAGE, collectionSlug);
@@ -420,6 +421,7 @@ public String addToCollection(
 	@PostMapping(path = SeriesUrl.INFO_SERIES_PAGE, params = "action=REMOVE")
 	public String removeFromCollection(
 		@PathVariable("id") Integer seriesId,
+		@RequestParam(name = "id", defaultValue = "0") Integer seriesInstanceId,
 		@AuthenticationPrincipal CustomUserDetails currentUserDetails,
 		RedirectAttributes redirectAttributes,
 		HttpServletResponse response)
@@ -430,14 +432,19 @@ public String removeFromCollection(
 			return null;
 		}
 
+		if (seriesInstanceId == 0) {
+			response.sendError(HttpServletResponse.SC_BAD_REQUEST);
+			return null;
+		}
+		
 		boolean seriesExists = seriesService.isSeriesExist(seriesId);
 		if (!seriesExists) {
 			response.sendError(HttpServletResponse.SC_NOT_FOUND);
 			return null;
 		}
 
 		Integer userId = currentUserDetails.getUserId();
-		collectionService.removeFromCollection(userId, seriesId);
+		collectionService.removeFromCollection(userId, seriesId, seriesInstanceId);
 
 		redirectAttributes.addFlashAttribute("justRemovedSeries", true);
 
@@ -644,10 +651,12 @@ public static void loadErrorsFromDownloadInterceptor(
 
 		boolean userCanAddImagesToSeries = isUserCanAddImagesToSeries(series);
 		model.put("allowAddingImages", userCanAddImagesToSeries);
-
-		boolean isSeriesInCollection =
-			collectionService.isSeriesInCollection(currentUserId, seriesId);
-		model.put("isSeriesInCollection", isSeriesInCollection);
+		
+		if (SecurityContextUtils.hasAuthority(Authority.UPDATE_COLLECTION)) {
+			Map<Integer, Integer> seriesInstances =
+				collectionService.findSeriesInstances(currentUserId, seriesId);
+			model.put("seriesInstances", seriesInstances);
+		}
 
 		if (SecurityContextUtils.hasAuthority(Authority.VIEW_SERIES_SALES)) {
 			List<PurchaseAndSaleDto> purchasesAndSales =

src/main/java/ru/mystamps/web/support/spring/jdbc/MapIntegerIntegerResultSetExtractor.java

@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2009-2019 Slava Semushin <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ */
+package ru.mystamps.web.support.spring.jdbc;
+
+import lombok.RequiredArgsConstructor;
+import org.springframework.dao.DataAccessException;
+import org.springframework.jdbc.core.ResultSetExtractor;
+import ru.mystamps.web.common.JdbcUtils;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.HashMap;
+import java.util.Map;
+
+@RequiredArgsConstructor
+public class MapIntegerIntegerResultSetExtractor
+	implements ResultSetExtractor<Map<Integer, Integer>> {
+	
+	private final String keyFieldName;
+	private final String valueFieldName;
+	
+	@Override
+	public Map<Integer, Integer> extractData(ResultSet rs)
+		throws SQLException, DataAccessException {
+		
+		Map<Integer, Integer> result = new HashMap<>();
+		
+		while (rs.next()) {
+			Integer key   = JdbcUtils.getInteger(rs, keyFieldName);
+			Integer value = JdbcUtils.getInteger(rs, valueFieldName);
+			result.put(key, value);
+		}
+		
+		return result;
+	}
+	
+}

src/main/resources/liquibase/version/0.4.2/2019-11-27--add_collections_series_id.xml

@@ -41,4 +41,14 @@
 
 	</changeSet>
 
+	<changeSet id="add-series-3-to-collection-once-more" author="php-coder" context="test-data">
+		
+		<insert tableName="collections_series">
+			<column name="collection_id" valueComputed="(SELECT id FROM collections WHERE slug = 'seriesowner')" />
+			<column name="series_id" valueComputed="(SELECT id FROM series WHERE quantity = 3 ORDER BY id LIMIT 1)" />
+			<column name="number_of_stamps" valueNumeric="2" />
+		</insert>
+		
+	</changeSet>
+	
 </databaseChangeLog>

src/main/resources/ru/mystamps/i18n/Messages.properties

@@ -132,12 +132,12 @@ t_pick_percent_name = Pick "%name%"
 t_series_info = Info about series
 t_yes = Yes
 t_no = No
-t_series_not_in_collection = Series isn't part of your collection
+t_series_in_collection = You already have this series. Add another one instance
+t_series_not_in_collection = You don't have this series. Add one instance
 t_i_have = I have
 t_only_for_paid_users = Only for paid users
 t_i_bought_for = I bought for
 t_out_of_n_stamps = out of {0} stamps
-t_series_in_collection = Series is part of your collection
 t_add_to_collection = Add to collection
 t_remove_from_collection = Remove from collection
 t_need_authentication_to_add_series_to_collection = \

src/main/resources/ru/mystamps/i18n/Messages_ru.properties

@@ -131,12 +131,12 @@ t_pick_percent_name = Выбрать "%name%"
 t_series_info = Информация о серии
 t_yes = Да
 t_no = Нет
-t_series_not_in_collection = Этой серии нет в вашей коллекции
+t_series_in_collection = Эта серия уже есть в вашей коллекции. Добавить еще один экземпляр
+t_series_not_in_collection = Этой серии нет в вашей коллекции. Добавить экземпляр
 t_i_have = У меня есть
 t_only_for_paid_users = Только для платных пользователей
 t_i_bought_for = Я купил за
 t_out_of_n_stamps = из {0} марок
-t_series_in_collection = Эта серия есть в вашей коллекции
 t_add_to_collection = Добавить в коллекцию
 t_remove_from_collection = Убрать из коллекции
 t_need_authentication_to_add_series_to_collection = \

src/main/resources/sql/collection_dao_queries.properties

@@ -100,6 +100,14 @@ SELECT COUNT(*) \
  WHERE c.user_id = :user_id \
    AND cs.series_id = :series_id
 
+collection.find_series_instances = \
+SELECT cs.id, cs.number_of_stamps \
+  FROM collections c \
+  JOIN collections_series cs \
+    ON cs.collection_id = c.id \
+ WHERE c.user_id = :user_id \
+   AND cs.series_id = :series_id
+
 collection.add_series_to_collection = \
 INSERT \
   INTO collections_series \
@@ -117,10 +125,10 @@ SELECT c.id AS collection_id \
   FROM collections c \
  WHERE c.user_id = :user_id
 
-collection.remove_series_from_collection = \
+collection.remove_series_instance_from_collection = \
 DELETE \
   FROM collections_series \
- WHERE series_id = :series_id \
+ WHERE id = :id \
    AND collection_id = ( \
          SELECT id \
            FROM collections \

src/main/webapp/WEB-INF/views/collection/info.html

@@ -141,7 +141,8 @@ <h4 class="panel-title" th:text="#{t_stamps_by_categories}">Stamps by categories
 										th:if="${series.quantity != series.numberOfStamps}"
 										th:text="#{t_m_out_of_n(${series.numberOfStamps}, ${series.quantity})}"></span>
 									/*/-->
-									<span th:if="${justAddedSeriesId != null and justAddedSeriesId == series.id}" class="label label-success">New</span>
+									<span th:if="${justAddedSeriesId != null and justAddedSeriesId == series.id and justAddedNumberOfStamps == series.NumberOfStamps}" class="label label-success">New</span>
+									<!--/* @todo #1123 /collection/{slug}: show "New" badge only once */-->
 								</li>
 								<li>
 									<a href="../category/info.html">Animals, Italy, 22&nbsp;stamps</a>