Skip to content

Commit 899ff5e

Browse files
php-coderphp-coder
committed
feat: users can add a comment to a series
Note also that the possibility to add a comment during a series creation has been removed. A series now can have more than one comment from different users and in order to not confuse users by the fact that a comment is visible only to them while other fields are visible all, this possibility was removed. Fix #1505
1 parent 7146267 commit 899ff5e

29 files changed

+261
-191
lines changed

‎NEWS.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
0.x (upcoming release)
2+
- (feature) users can add a comment to a series
23
(infrastructure) migrate to JUnit 5
34

45
0.4.5

‎docs/diagrams/all-use-cases.puml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,10 @@
6565
---- auto-filling category/country from URL (U)
6666
---- category/country suggestion (U)
6767
---- with an image from URL (AD)
68-
---- with a comment (AD)
6968
*** **add** a year (U)
7069
*** **add** catalog numbers (U)
7170
*** **add** a catalog price (U)
72-
*** **add** a comment (AD)
71+
*** **add** a comment (U)
7372
*** **add** an image (AD and an owner)
7473
*** **add** an image from URL (AD)
7574
*** **replace** an image (AD)

‎src/main/config/spotbugs-filter.xml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@
9494
</Or>
9595
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
9696
</Match>
97+
<Match>
98+
<Class name="ru.mystamps.web.feature.series.AddCommentDbDto" />
99+
<Or>
100+
<Method name="getCreatedAt" />
101+
<Method name="setCreatedAt" />
102+
<Method name="getUpdatedAt" />
103+
<Method name="setUpdatedAt" />
104+
</Or>
105+
<Bug pattern="EI_EXPOSE_REP,EI_EXPOSE_REP2" />
106+
</Match>
97107
<Match>
98108
<Class name="ru.mystamps.web.feature.series.AddReleaseYearDbDto" />
99109
<Or>

‎src/main/java/ru/mystamps/web/feature/series/AddCommentDbDto.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/*
2+
* Copyright (C) 2009-2021 Slava Semushin <[email protected]>
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License as published by
6+
* the Free Software Foundation; either version 2 of the License, or
7+
* (at your option) any later version.
8+
*
9+
* This program is distributed in the hope that it will be useful,
10+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
* GNU General Public License for more details.
13+
*
14+
* You should have received a copy of the GNU General Public License
15+
* along with this program; if not, write to the Free Software
16+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17+
*/
18+
package ru.mystamps.web.feature.series;
19+
20+
import lombok.Getter;
21+
import lombok.Setter;
22+
23+
import java.util.Date;
24+
25+
@Getter
26+
@Setter
27+
public class AddCommentDbDto {
28+
private Integer seriesId;
29+
private Integer userId;
30+
private String comment;
31+
private Date createdAt;
32+
private Date updatedAt;
33+
}

‎src/main/java/ru/mystamps/web/feature/series/AddSeriesDbDto.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
@Getter
2828
@Setter
29-
@ToString(exclude = { "comment", "createdAt", "createdBy", "updatedAt", "updatedBy" })
29+
@ToString(exclude = { "createdAt", "createdBy", "updatedAt", "updatedBy" })
3030
@SuppressWarnings("PMD.TooManyFields")
3131
public class AddSeriesDbDto {
3232
private Integer categoryId;
@@ -46,8 +46,6 @@ public class AddSeriesDbDto {
4646
private Integer releaseMonth;
4747
private Integer releaseYear;
4848

49-
private String comment;
50-
5149
private Date createdAt;
5250
private Integer createdBy;
5351
private Date updatedAt;

‎src/main/java/ru/mystamps/web/feature/series/AddSeriesDto.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,5 @@ public interface AddSeriesDto {
4949
String getZagorskiNumbers();
5050
BigDecimal getZagorskiPrice();
5151

52-
String getComment();
5352
MultipartFile getImage();
5453
}

‎src/main/java/ru/mystamps/web/feature/series/AddSeriesForm.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,11 @@
3838
import javax.validation.constraints.Max;
3939
import javax.validation.constraints.Min;
4040
import javax.validation.constraints.NotNull;
41-
import javax.validation.constraints.Size;
4241
import java.math.BigDecimal;
4342

4443
import static ru.mystamps.web.feature.image.ImageValidation.MAX_IMAGE_SIZE;
4544
import static ru.mystamps.web.feature.series.SeriesValidation.MAX_DAYS_IN_MONTH;
4645
import static ru.mystamps.web.feature.series.SeriesValidation.MAX_MONTHS_IN_YEAR;
47-
import static ru.mystamps.web.feature.series.SeriesValidation.MAX_SERIES_COMMENT_LENGTH;
4846
import static ru.mystamps.web.feature.series.SeriesValidation.MAX_STAMPS_IN_SERIES;
4947
import static ru.mystamps.web.feature.series.SeriesValidation.MIN_RELEASE_YEAR;
5048
import static ru.mystamps.web.feature.series.SeriesValidation.MIN_STAMPS_IN_SERIES;
@@ -142,9 +140,6 @@ public class AddSeriesForm implements AddSeriesDto, HasImageOrImageUrl, Nullable
142140
@Price
143141
private BigDecimal zagorskiPrice;
144142

145-
@Size(max = MAX_SERIES_COMMENT_LENGTH, message = "{value.too-long}")
146-
private String comment;
147-
148143
// Name of this field should match with the value of
149144
// DownloadImageInterceptor.UPLOADED_IMAGE_FIELD_NAME.
150145
@NotEmptyFilename(groups = RequireImageCheck.class)

‎src/main/java/ru/mystamps/web/feature/series/JdbcSeriesDao.java

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ public Integer add(AddSeriesDbDto series) {
132132
params.put("gibbons_price", series.getGibbonsPrice());
133133
params.put("solovyov_price", series.getSolovyovPrice());
134134
params.put("zagorski_price", series.getZagorskiPrice());
135-
params.put("comment", series.getComment());
136135
params.put("created_at", series.getCreatedAt());
137136
params.put("created_by", series.getCreatedBy());
138137
params.put("updated_at", series.getUpdatedAt());
@@ -157,17 +156,20 @@ public Integer add(AddSeriesDbDto series) {
157156
}
158157

159158
@Override
160-
public void addComment(Integer seriesId, String comment) {
159+
public void addComment(AddCommentDbDto dto) {
161160
Map<String, Object> params = new HashMap<>();
162-
params.put("series_id", seriesId);
163-
params.put("comment", comment);
161+
params.put("series_id", dto.getSeriesId());
162+
params.put("user_id", dto.getUserId());
163+
params.put("comment", dto.getComment());
164+
params.put("created_at", dto.getCreatedAt());
165+
params.put("updated_at", dto.getUpdatedAt());
164166

165167
int affected = jdbcTemplate.update(addCommentSql, params);
166168

167169
// @todo #785 Update series: handle refuse to update an existing comment gracefully
168170
Validate.validState(
169171
affected == 1,
170-
"Unexpected number of affected rows after updating series: %d",
172+
"Unexpected number of affected rows after adding a series comment: %d",
171173
affected
172174
);
173175
}
@@ -239,10 +241,12 @@ public List<SeriesLinkDto> findLastAdded(int quantity, String lang) {
239241
return jdbcTemplate.query(findLastAddedSeriesSql, params, RowMappers::forSeriesLinkDto);
240242
}
241243

244+
// CheckStyle: ignore LineLength for next 2 lines
242245
@Override
243-
public SeriesFullInfoDto findByIdAsSeriesFullInfo(Integer seriesId, String lang) {
246+
public SeriesFullInfoDto findByIdAsSeriesFullInfo(Integer seriesId, Integer userId, String lang) {
244247
Map<String, Object> params = new HashMap<>();
245248
params.put("series_id", seriesId);
249+
params.put("user_id", userId);
246250
params.put("lang", lang);
247251

248252
try {

‎src/main/java/ru/mystamps/web/feature/series/RestSeriesController.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public ResponseEntity<Void> updateSeries(
7373
String path = patch.getPath();
7474
switch (path) {
7575
case "/comment":
76-
seriesService.addComment(seriesId, patch.getValue());
76+
seriesService.addComment(seriesId, patch.getValue(), currentUserId);
7777
break;
7878
case "/release_year":
7979
seriesService.addReleaseYear(seriesId, patch.integerValue(), currentUserId);

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

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ protected void initSeriesFormBinder(WebDataBinder binder) {
117117
binder.registerCustomEditor(String.class, "gibbonsNumbers", editor);
118118
binder.registerCustomEditor(String.class, "solovyovNumbers", editor);
119119
binder.registerCustomEditor(String.class, "zagorskiNumbers", editor);
120-
binder.registerCustomEditor(String.class, "comment", new StringTrimmerEditor(true));
121120
}
122121

123122
@InitBinder("addSeriesSalesForm")
@@ -194,10 +193,7 @@ public String processInput(
194193
return null;
195194
}
196195

197-
boolean userCanAddComments = SecurityContextUtils.hasAuthority(
198-
Authority.ADD_COMMENTS_TO_SERIES
199-
);
200-
Integer seriesId = seriesService.add(form, currentUserId, userCanAddComments);
196+
Integer seriesId = seriesService.add(form, currentUserId);
201197

202198
return redirectTo(SeriesUrl.INFO_SERIES_PAGE, seriesId);
203199
}
@@ -220,7 +216,12 @@ public String showInfo(
220216
boolean userCanSeeHiddenImages = SecurityContextUtils.hasAuthority(
221217
Authority.VIEW_HIDDEN_IMAGES
222218
);
223-
SeriesDto series = seriesService.findFullInfoById(seriesId, lang, userCanSeeHiddenImages);
219+
SeriesDto series = seriesService.findFullInfoById(
220+
seriesId,
221+
currentUserId,
222+
lang,
223+
userCanSeeHiddenImages
224+
);
224225
if (series == null) {
225226
response.sendError(HttpServletResponse.SC_NOT_FOUND);
226227
return null;
@@ -372,7 +373,12 @@ public String processImage(
372373
boolean userCanSeeHiddenImages = SecurityContextUtils.hasAuthority(
373374
Authority.VIEW_HIDDEN_IMAGES
374375
);
375-
SeriesDto series = seriesService.findFullInfoById(seriesId, lang, userCanSeeHiddenImages);
376+
SeriesDto series = seriesService.findFullInfoById(
377+
seriesId,
378+
currentUserId,
379+
lang,
380+
userCanSeeHiddenImages
381+
);
376382
if (series == null) {
377383
response.sendError(HttpServletResponse.SC_NOT_FOUND);
378384
return null;
@@ -448,6 +454,7 @@ public String addToCollection(
448454
);
449455
SeriesDto series = seriesService.findFullInfoById(
450456
seriesId,
457+
userId,
451458
lang,
452459
userCanSeeHiddenImages
453460
);
@@ -530,7 +537,12 @@ public String processAskForm(
530537
boolean userCanSeeHiddenImages = SecurityContextUtils.hasAuthority(
531538
Authority.VIEW_HIDDEN_IMAGES
532539
);
533-
SeriesDto series = seriesService.findFullInfoById(seriesId, lang, userCanSeeHiddenImages);
540+
SeriesDto series = seriesService.findFullInfoById(
541+
seriesId,
542+
currentUserId,
543+
lang,
544+
userCanSeeHiddenImages
545+
);
534546
if (series == null) {
535547
response.sendError(HttpServletResponse.SC_NOT_FOUND);
536548
return null;

‎src/main/java/ru/mystamps/web/feature/series/SeriesDao.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@
2424
@SuppressWarnings("PMD.TooManyMethods")
2525
public interface SeriesDao {
2626
Integer add(AddSeriesDbDto series);
27-
void addComment(Integer seriesId, String comment);
27+
void addComment(AddCommentDbDto dto);
2828
void addReleaseYear(AddReleaseYearDbDto dto);
2929
void markAsModified(Integer seriesId, Date updateAt, Integer updatedBy);
3030
List<SitemapInfoDto> findAllForSitemap();
3131
List<SeriesLinkDto> findSimilarSeries(Integer seriesId, String lang);
3232
List<SeriesLinkDto> findLastAdded(int quantity, String lang);
33-
SeriesFullInfoDto findByIdAsSeriesFullInfo(Integer seriesId, String lang);
33+
SeriesFullInfoDto findByIdAsSeriesFullInfo(Integer seriesId, Integer userId, String lang);
3434
List<SeriesInfoDto> findByIdsAsSeriesInfo(List<Integer> seriesIds, String lang);
3535
List<SeriesInfoDto> findByCategorySlugAsSeriesInfo(String slug, String lang);
3636
List<SeriesInGalleryDto> findByCountrySlug(String slug, String lang);

‎src/main/java/ru/mystamps/web/feature/series/SeriesService.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
// FIXME: move stamps related methods to separate interface (#88)
2525
@SuppressWarnings("PMD.TooManyMethods")
2626
public interface SeriesService {
27-
Integer add(AddSeriesDto dto, Integer userId, boolean userCanAddComments);
28-
void addComment(Integer seriesId, String comment);
27+
Integer add(AddSeriesDto dto, Integer userId);
28+
void addComment(Integer seriesId, String comment, Integer userId);
2929
void addReleaseYear(Integer seriesId, Integer year, Integer userId);
3030
void addCatalogPrice(StampsCatalog catalog, Integer seriesId, BigDecimal price, Integer userId);
3131
void addCatalogNumbers(StampsCatalog catalog, Integer seriesId, String numbers, Integer userId);
@@ -38,7 +38,12 @@ public interface SeriesService {
3838
boolean isSeriesExist(Integer seriesId);
3939
Integer findQuantityById(Integer seriesId);
4040

41-
SeriesDto findFullInfoById(Integer seriesId, String lang, boolean userCanSeeHiddenImages);
41+
SeriesDto findFullInfoById(
42+
Integer seriesId,
43+
Integer userId,
44+
String lang,
45+
boolean userCanSeeHiddenImages
46+
);
4247

4348
List<SeriesInfoDto> findByMichelNumber(String michelNumberCode, String lang);
4449
List<SeriesInfoDto> findByScottNumber(String scottNumberCode, String lang);

‎src/main/java/ru/mystamps/web/feature/series/SeriesServiceImpl.java

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Date;
3232
import java.util.List;
3333
import java.util.Locale;
34+
import java.util.Optional;
3435
import java.util.Set;
3536

3637
// FIXME: move stamps related methods to separate interface (#88)
@@ -53,7 +54,7 @@ public class SeriesServiceImpl implements SeriesService {
5354
@Override
5455
@Transactional
5556
@PreAuthorize(HasAuthority.CREATE_SERIES)
56-
public Integer add(AddSeriesDto dto, Integer userId, boolean userCanAddComments) {
57+
public Integer add(AddSeriesDto dto, Integer userId) {
5758
Validate.isTrue(dto != null, "DTO must be non null");
5859
Validate.isTrue(dto.getQuantity() != null, "Quantity must be non null");
5960
Validate.isTrue(dto.getPerforated() != null, "Perforated property must be non null");
@@ -79,15 +80,6 @@ public Integer add(AddSeriesDto dto, Integer userId, boolean userCanAddComments)
7980
series.setSolovyovPrice(dto.getSolovyovPrice());
8081
series.setZagorskiPrice(dto.getZagorskiPrice());
8182

82-
if (userCanAddComments && dto.getComment() != null) {
83-
Validate.isTrue(
84-
!dto.getComment().trim().isEmpty(),
85-
"Comment must be non empty"
86-
);
87-
88-
series.setComment(dto.getComment());
89-
}
90-
9183
Date now = new Date();
9284
series.setCreatedAt(now);
9385
series.setUpdatedAt(now);
@@ -123,17 +115,26 @@ public Integer add(AddSeriesDto dto, Integer userId, boolean userCanAddComments)
123115
@Override
124116
@Transactional
125117
@PreAuthorize(HasAuthority.ADD_COMMENTS_TO_SERIES)
126-
public void addComment(Integer seriesId, String comment) {
118+
public void addComment(Integer seriesId, String comment, Integer userId) {
127119
Validate.isTrue(seriesId != null, "Series id must be non null");
128120
Validate.isTrue(comment != null, "Comment must be non null");
129121
Validate.isTrue(!comment.trim().isEmpty(), "Comment must be non empty");
122+
Validate.isTrue(userId != null, "User id must be non null");
130123

131-
// We don't touch updated_at/updated_by fields because:
132-
// - a comment is a meta information that is visible only by admins.
133-
// From user's point of view, this field doesn't exist
124+
// We don't need to modify series.updated_at/updated_by fields because:
125+
// - a comment is a meta information that is invisible publicly
134126
// - updated_at field is used by logic for sitemap.xml generation
135127
// and we don't want to affect this
136-
seriesDao.addComment(seriesId, comment);
128+
AddCommentDbDto dto = new AddCommentDbDto();
129+
dto.setSeriesId(seriesId);
130+
dto.setUserId(userId);
131+
dto.setComment(comment);
132+
133+
Date now = new Date();
134+
dto.setCreatedAt(now);
135+
dto.setUpdatedAt(now);
136+
137+
seriesDao.addComment(dto);
137138

138139
log.info("Series #{}: a comment has been added", seriesId);
139140
}
@@ -307,12 +308,18 @@ public Integer findQuantityById(Integer seriesId) {
307308
@Transactional(readOnly = true)
308309
public SeriesDto findFullInfoById(
309310
Integer seriesId,
311+
Integer userId,
310312
String lang,
311313
boolean userCanSeeHiddenImages) {
312314

313315
Validate.isTrue(seriesId != null, "Series id must be non null");
314316

315-
SeriesFullInfoDto seriesBaseInfo = seriesDao.findByIdAsSeriesFullInfo(seriesId, lang);
317+
// @todo #1505 Don't load a series comment for anonymous users
318+
SeriesFullInfoDto seriesBaseInfo = seriesDao.findByIdAsSeriesFullInfo(
319+
seriesId,
320+
Optional.ofNullable(userId).orElse(0),
321+
lang
322+
);
316323
if (seriesBaseInfo == null) {
317324
return null;
318325
}

‎src/main/java/ru/mystamps/web/feature/series/importing/ImportSeriesForm.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,4 @@ public BigDecimal getZagorskiPrice() {
178178
return null;
179179
}
180180

181-
@Override
182-
public String getComment() {
183-
return null;
184-
}
185-
186181
}

‎src/main/java/ru/mystamps/web/feature/series/importing/SeriesImportServiceImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public Integer addSeries(
100100
Integer requestId,
101101
Integer userId) {
102102

103-
Integer seriesId = seriesService.add(dto, userId, false);
103+
Integer seriesId = seriesService.add(dto, userId);
104104

105105
if (saleDto != null) {
106106
if (saleDto.getSellerId() == null && sellerDto != null) {

0 commit comments

Comments
 (0)