Allow deletion of forum replies #111

Open
ansv7779 wants to merge 5 commits from remove-forum-post into develop
13 changed files with 243 additions and 27 deletions

View File

@ -348,14 +348,16 @@ public class CoreConfig {
ForumPostReadStateRepository readStateRepository, ForumPostReadStateRepository readStateRepository,
AbstractThreadRepository threadRepository, AbstractThreadRepository threadRepository,
FileService fileService, FileService fileService,
EventBus eventBus EventBus eventBus,
CurrentUser currentUser
) { ) {
return new BasicForumServiceImpl( return new BasicForumServiceImpl(
forumPostRepository, forumPostRepository,
readStateRepository, readStateRepository,
threadRepository, threadRepository,
fileService, fileService,
eventBus eventBus,
currentUser
); );
} }

View File

@ -23,4 +23,12 @@ public interface BasicForumService extends Serializable {
ForumThread createThread(String subject); ForumThread createThread(String subject);
long countUnreadThreads(List<ForumThread> forumThreadList, User user); long countUnreadThreads(List<ForumThread> forumThreadList, User user);
ForumPost getLastPost(ForumThread forumThread);
boolean hasAttachments(ForumThread forumThread);
boolean canDelete(ForumPost forumPost);
void deletePost(ForumPost post);
} }

View File

@ -10,6 +10,7 @@ import se.su.dsv.scipro.file.FileService;
import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.forum.dataobjects.ForumPostReadState; import se.su.dsv.scipro.forum.dataobjects.ForumPostReadState;
import se.su.dsv.scipro.forum.dataobjects.ForumThread; import se.su.dsv.scipro.forum.dataobjects.ForumThread;
import se.su.dsv.scipro.system.CurrentUser;
import se.su.dsv.scipro.system.User; import se.su.dsv.scipro.system.User;
public class BasicForumServiceImpl implements BasicForumService { public class BasicForumServiceImpl implements BasicForumService {
@ -19,6 +20,7 @@ public class BasicForumServiceImpl implements BasicForumService {
private final ForumPostReadStateRepository readStateRepository; private final ForumPostReadStateRepository readStateRepository;
private final FileService fileService; private final FileService fileService;
private final EventBus eventBus; private final EventBus eventBus;
private final CurrentUser currentUserProvider;
@Inject @Inject
public BasicForumServiceImpl( public BasicForumServiceImpl(
@ -26,13 +28,15 @@ public class BasicForumServiceImpl implements BasicForumService {
final ForumPostReadStateRepository readStateRepository, final ForumPostReadStateRepository readStateRepository,
AbstractThreadRepository threadRepository, AbstractThreadRepository threadRepository,
final FileService fileService, final FileService fileService,
final EventBus eventBus final EventBus eventBus,
final CurrentUser currentUserProvider
) { ) {
this.postRepository = postRepository; this.postRepository = postRepository;
this.readStateRepository = readStateRepository; this.readStateRepository = readStateRepository;
this.threadRepository = threadRepository; this.threadRepository = threadRepository;
this.fileService = fileService; this.fileService = fileService;
this.eventBus = eventBus; this.eventBus = eventBus;
this.currentUserProvider = currentUserProvider;
} }
@Override @Override
@ -66,7 +70,7 @@ public class BasicForumServiceImpl implements BasicForumService {
@Override @Override
public boolean isThreadRead(User user, ForumThread forumThread) { public boolean isThreadRead(User user, ForumThread forumThread) {
for (ForumPost post : forumThread.getPosts()) { for (ForumPost post : getPosts(forumThread)) {
if (!getReadState(user, post).isRead()) { if (!getReadState(user, post).isRead()) {
return false; return false;
} }
@ -133,4 +137,56 @@ public class BasicForumServiceImpl implements BasicForumService {
return post; return post;
} }
@Override
public ForumPost getLastPost(ForumThread forumThread) {
return Collections.max(
getPosts(forumThread),
Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId)
);
}
@Override
public boolean hasAttachments(ForumThread forumThread) {
for (ForumPost post : getPosts(forumThread)) {
if (!post.getAttachments().isEmpty()) {
return true;
}
}
return false;
}
@Override
public boolean canDelete(ForumPost forumPost) {
ForumPost initialPost = forumPost.getForumThread().getPosts().get(0);
if (forumPost.equals(initialPost)) {
// The initial post in a thread can never be deleted
return false;
}
User user = currentUserProvider.get();
// Current user can be null meaning the call came from the system
if (user == null) {
// Allow the system to delete any post
return true;
}
return Objects.equals(forumPost.getPostedBy(), user);
}
@Override
@Transactional
public void deletePost(ForumPost post) {
if (!canDelete(post)) {
throw new PostCantBeDeletedException();
}
post.setDeleted(true);
postRepository.save(post);
}
private static final class PostCantBeDeletedException extends IllegalArgumentException {
public PostCantBeDeletedException() {
super("User is not allowed to delete post");
}
}
} }

View File

@ -116,13 +116,4 @@ public class ForumThread extends LazyDeletableDomainObject {
public User getCreatedBy() { public User getCreatedBy() {
return getPosts().get(0).getPostedBy(); return getPosts().get(0).getPostedBy();
} }
public boolean hasAttachments() {
for (ForumPost post : posts) {
if (!post.getAttachments().isEmpty()) {
return true;
}
}
return false;
}
} }

View File

@ -121,6 +121,8 @@ public class BasicForumServiceImplTest {
ForumThread forumThread = new ForumThread(); ForumThread forumThread = new ForumThread();
forumThread.addPost(post); forumThread.addPost(post);
when(postRepository.findByThread(forumThread)).thenReturn(List.of(post));
when(readStateRepository.find(eq(goodUser), isA(ForumPost.class))).thenReturn(readState); when(readStateRepository.find(eq(goodUser), isA(ForumPost.class))).thenReturn(readState);
when(readStateRepository.find(eq(badUser), isA(ForumPost.class))).thenReturn(notReadState); when(readStateRepository.find(eq(badUser), isA(ForumPost.class))).thenReturn(notReadState);

View File

@ -0,0 +1,83 @@
package se.su.dsv.scipro.forum;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import jakarta.inject.Inject;
import java.util.Set;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.forum.dataobjects.ForumThread;
import se.su.dsv.scipro.system.User;
import se.su.dsv.scipro.test.IntegrationTest;
public class BasicForumServiceIntegrationTest extends IntegrationTest {
@Inject
BasicForumService basicForumService;
private User op;
private User commenter;
@BeforeEach
public void setUp() {
User op = User.builder().firstName("Bill").lastName("Gates").emailAddress("bill@example.com").build();
this.op = save(op);
User commenter = User.builder().firstName("Steve").lastName("Jobs").emailAddress("steve@example.com").build();
this.commenter = save(commenter);
}
@Test
public void can_not_delete_original_post() {
ForumThread thread = basicForumService.createThread("Test thread");
ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
setLoggedInAs(op);
assertFalse(basicForumService.canDelete(originalPost));
assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(originalPost));
}
@Test
public void can_delete_reply_to_original_post() {
ForumThread thread = basicForumService.createThread("Test thread");
ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
setLoggedInAs(commenter);
assertTrue(basicForumService.canDelete(reply));
assertDoesNotThrow(() -> basicForumService.deletePost(reply));
}
@Test
public void can_not_delete_someone_elses_reply() {
ForumThread thread = basicForumService.createThread("Test thread");
ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
setLoggedInAs(op);
assertFalse(basicForumService.canDelete(reply));
assertThrows(IllegalArgumentException.class, () -> basicForumService.deletePost(reply));
}
@Test
public void system_can_delete_all_replies() {
ForumThread thread = basicForumService.createThread("Test thread");
ForumPost originalPost = basicForumService.createReply(thread, op, "Test post", Set.of());
ForumPost reply = basicForumService.createReply(thread, commenter, "Test reply", Set.of());
ForumPost secondReply = basicForumService.createReply(thread, op, "Test post", Set.of());
setLoggedInAs(null);
assertTrue(basicForumService.canDelete(reply));
assertDoesNotThrow(() -> basicForumService.deletePost(reply));
assertTrue(basicForumService.canDelete(secondReply));
assertDoesNotThrow(() -> basicForumService.deletePost(secondReply));
}
}

View File

@ -1,5 +1,6 @@
package se.su.dsv.scipro.test; package se.su.dsv.scipro.test;
import jakarta.inject.Inject;
import jakarta.persistence.EntityManager; import jakarta.persistence.EntityManager;
import jakarta.persistence.EntityManagerFactory; import jakarta.persistence.EntityManagerFactory;
import jakarta.persistence.EntityTransaction; import jakarta.persistence.EntityTransaction;
@ -25,6 +26,7 @@ import se.su.dsv.scipro.RepositoryConfiguration;
import se.su.dsv.scipro.profiles.CurrentProfile; import se.su.dsv.scipro.profiles.CurrentProfile;
import se.su.dsv.scipro.sukat.Sukat; import se.su.dsv.scipro.sukat.Sukat;
import se.su.dsv.scipro.system.CurrentUser; import se.su.dsv.scipro.system.CurrentUser;
import se.su.dsv.scipro.system.User;
@Testcontainers @Testcontainers
public abstract class SpringTest { public abstract class SpringTest {
@ -35,6 +37,9 @@ public abstract class SpringTest {
@Container @Container
static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11"); static MariaDBContainer<?> mariaDBContainer = new MariaDBContainer<>("mariadb:10.11");
@Inject
private TestUser testUser;
@BeforeEach @BeforeEach
public final void prepareSpring() throws SQLException { public final void prepareSpring() throws SQLException {
MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl()); MariaDbDataSource dataSource = new MariaDbDataSource(mariaDBContainer.getJdbcUrl());
@ -56,6 +61,8 @@ public abstract class SpringTest {
annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager); annotationConfigApplicationContext.getBeanFactory().registerSingleton("entityManager", this.entityManager);
annotationConfigApplicationContext.refresh(); annotationConfigApplicationContext.refresh();
annotationConfigApplicationContext.getAutowireCapableBeanFactory().autowireBean(this); annotationConfigApplicationContext.getAutowireCapableBeanFactory().autowireBean(this);
testUser.setUser(null); // default to system
} }
@AfterEach @AfterEach
@ -75,6 +82,10 @@ public abstract class SpringTest {
} }
} }
protected void setLoggedInAs(User user) {
this.testUser.setUser(user);
}
@Configuration(proxyBeanMethods = false) @Configuration(proxyBeanMethods = false)
@Import({ CoreConfig.class, RepositoryConfiguration.class }) @Import({ CoreConfig.class, RepositoryConfiguration.class })
public static class TestContext { public static class TestContext {
@ -96,7 +107,7 @@ public abstract class SpringTest {
@Bean @Bean
public CurrentUser currentUser() { public CurrentUser currentUser() {
return () -> null; return new TestUser();
} }
@Bean @Bean
@ -106,4 +117,18 @@ public abstract class SpringTest {
return currentProfile; return currentProfile;
} }
} }
private static class TestUser implements CurrentUser {
private User user;
@Override
public User get() {
return user;
}
private void setUser(User user) {
this.user = user;
}
}
} }

View File

@ -7,9 +7,10 @@
<body> <body>
<wicket:panel> <wicket:panel>
<div class="messageWrap"> <div class="messageWrap">
<div class="forumBlueBackground"> <div class="forumBlueBackground d-flex justify-content-between">
<!-- DATE ROW--> <!-- DATE ROW-->
<wicket:container wicket:id="dateCreated"/> <span wicket:id="dateCreated"></span>
<button wicket:id="delete" class="btn btn-sm btn-outline-danger ms-auto">Delete</button>
</div> </div>
<div class="forumGrayBackground"> <div class="forumGrayBackground">
<div class="vertAlign"> <div class="vertAlign">

View File

@ -1,7 +1,9 @@
package se.su.dsv.scipro.forum.panels.threaded; package se.su.dsv.scipro.forum.panels.threaded;
import jakarta.inject.Inject;
import org.apache.wicket.Component; import org.apache.wicket.Component;
import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.WebMarkupContainer;
import org.apache.wicket.markup.html.link.Link;
import org.apache.wicket.markup.html.panel.Panel; import org.apache.wicket.markup.html.panel.Panel;
import org.apache.wicket.model.IModel; import org.apache.wicket.model.IModel;
import org.apache.wicket.model.LambdaModel; import org.apache.wicket.model.LambdaModel;
@ -11,9 +13,11 @@ import se.su.dsv.scipro.components.ListAdapterModel;
import se.su.dsv.scipro.components.SmarterLinkMultiLineLabel; import se.su.dsv.scipro.components.SmarterLinkMultiLineLabel;
import se.su.dsv.scipro.data.enums.DateStyle; import se.su.dsv.scipro.data.enums.DateStyle;
import se.su.dsv.scipro.file.FileReference; import se.su.dsv.scipro.file.FileReference;
import se.su.dsv.scipro.forum.BasicForumService;
import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.profile.UserLinkPanel; import se.su.dsv.scipro.profile.UserLinkPanel;
import se.su.dsv.scipro.repository.panels.ViewAttachmentPanel; import se.su.dsv.scipro.repository.panels.ViewAttachmentPanel;
import se.su.dsv.scipro.session.SciProSession;
public class ForumPostPanel extends Panel { public class ForumPostPanel extends Panel {
@ -22,6 +26,9 @@ public class ForumPostPanel extends Panel {
public static final String CONTENT = "content"; public static final String CONTENT = "content";
public static final String ATTACHMENT = "attachment"; public static final String ATTACHMENT = "attachment";
@Inject
private BasicForumService basicForumService;
public ForumPostPanel(String id, final IModel<ForumPost> model) { public ForumPostPanel(String id, final IModel<ForumPost> model) {
super(id); super(id);
add(new UserLinkPanel(POSTED_BY, LambdaModel.of(model, ForumPost::getPostedBy, ForumPost::setPostedBy))); add(new UserLinkPanel(POSTED_BY, LambdaModel.of(model, ForumPost::getPostedBy, ForumPost::setPostedBy)));
@ -62,5 +69,28 @@ public class ForumPostPanel extends Panel {
} }
} }
); );
add(
new Link<>("delete", model) {
@Override
public void onClick() {
ForumPost post = getModelObject();
basicForumService.deletePost(post);
onPostDeleted();
}
@Override
protected void onConfigure() {
super.onConfigure();
setVisible(allowDeletion() && basicForumService.canDelete(getModelObject()));
}
}
);
} }
protected boolean allowDeletion() {
return true;
}
protected void onPostDeleted() {}
} }

View File

@ -1,8 +1,7 @@
package se.su.dsv.scipro.forum.panels.threaded; package se.su.dsv.scipro.forum.panels.threaded;
import jakarta.inject.Inject;
import java.io.Serializable; import java.io.Serializable;
import java.util.Collections;
import java.util.Comparator;
import java.util.List; import java.util.List;
import org.apache.wicket.markup.html.WebMarkupContainer; import org.apache.wicket.markup.html.WebMarkupContainer;
import org.apache.wicket.markup.html.basic.Label; import org.apache.wicket.markup.html.basic.Label;
@ -15,6 +14,7 @@ import org.apache.wicket.model.LambdaModel;
import org.apache.wicket.model.LoadableDetachableModel; import org.apache.wicket.model.LoadableDetachableModel;
import se.su.dsv.scipro.components.DateLabel; import se.su.dsv.scipro.components.DateLabel;
import se.su.dsv.scipro.data.enums.DateStyle; import se.su.dsv.scipro.data.enums.DateStyle;
import se.su.dsv.scipro.forum.BasicForumService;
import se.su.dsv.scipro.forum.Discussable; import se.su.dsv.scipro.forum.Discussable;
import se.su.dsv.scipro.forum.dataobjects.ForumPost; import se.su.dsv.scipro.forum.dataobjects.ForumPost;
import se.su.dsv.scipro.forum.dataobjects.ForumThread; import se.su.dsv.scipro.forum.dataobjects.ForumThread;
@ -23,6 +23,9 @@ import se.su.dsv.scipro.system.User;
public class ThreadsOverviewPanel<A> extends Panel { public class ThreadsOverviewPanel<A> extends Panel {
@Inject
private BasicForumService basicForumService;
public ThreadsOverviewPanel( public ThreadsOverviewPanel(
final String id, final String id,
final IModel<List<A>> model, final IModel<List<A>> model,
@ -41,7 +44,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
@Override @Override
protected void onConfigure() { protected void onConfigure() {
super.onConfigure(); super.onConfigure();
setVisibilityAllowed(discussion.getObject().hasAttachments()); setVisibilityAllowed(basicForumService.hasAttachments(discussion.getObject()));
} }
} }
); );
@ -80,7 +83,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
BookmarkablePageLink<Void> newThreadLink(String id, IModel<A> thread); BookmarkablePageLink<Void> newThreadLink(String id, IModel<A> thread);
} }
private static class LastPostColumn extends WebMarkupContainer { private class LastPostColumn extends WebMarkupContainer {
public LastPostColumn(String id, final IModel<ForumThread> model) { public LastPostColumn(String id, final IModel<ForumThread> model) {
super(id); super(id);
@ -110,10 +113,7 @@ public class ThreadsOverviewPanel<A> extends Panel {
return new LoadableDetachableModel<>() { return new LoadableDetachableModel<>() {
@Override @Override
protected ForumPost load() { protected ForumPost load() {
return Collections.max( return basicForumService.getLastPost(model.getObject());
model.getObject().getPosts(),
Comparator.comparing(ForumPost::getDateCreated).thenComparing(ForumPost::getId)
);
} }
}; };
} }

View File

@ -58,7 +58,16 @@ public class ViewForumThreadPanel<A> extends GenericPanel<A> {
new ListView<>(POST_LIST, new PostProvider()) { new ListView<>(POST_LIST, new PostProvider()) {
@Override @Override
protected void populateItem(ListItem<ForumPost> item) { protected void populateItem(ListItem<ForumPost> item) {
item.add(new ForumPostPanel(POST, item.getModel())); ListView<ForumPost> listView = this;
item.add(
new ForumPostPanel(POST, item.getModel()) {
@Override
protected void onPostDeleted() {
// Refresh the list of posts
listView.detach();
}
}
);
} }
} }
); );

View File

@ -22,7 +22,13 @@ interface Event {
@Override @Override
public Component component(String id, IModel<Event> model) { public Component component(String id, IModel<Event> model) {
return new ForumPostPanel(id, model.map(Message.class::cast).map(m -> m.forumPost)); return new ForumPostPanel(id, model.map(Message.class::cast).map(m -> m.forumPost)) {
@Override
protected boolean allowDeletion() {
// Do not allow deleting forum posts in the timeline
return false;
}
};
} }
@Override @Override

View File

@ -9,6 +9,7 @@ import org.apache.wicket.model.Model;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;
import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.junit.jupiter.MockitoExtension;
import se.su.dsv.scipro.SciProTest; import se.su.dsv.scipro.SciProTest;
import se.su.dsv.scipro.forum.Discussable; import se.su.dsv.scipro.forum.Discussable;
@ -20,11 +21,13 @@ import se.su.dsv.scipro.system.User;
public class ThreadsOverviewPanelTest extends SciProTest { public class ThreadsOverviewPanelTest extends SciProTest {
private List<ForumThread> threads; private List<ForumThread> threads;
private ForumPost post;
@BeforeEach @BeforeEach
public void setUp() throws Exception { public void setUp() throws Exception {
ForumThread forumThread = createThread(); ForumThread forumThread = createThread();
threads = Arrays.asList(forumThread); threads = Arrays.asList(forumThread);
Mockito.when(basicForumService.getLastPost(forumThread)).thenReturn(post);
} }
@Test @Test
@ -54,7 +57,7 @@ public class ThreadsOverviewPanelTest extends SciProTest {
private ForumThread createThread() { private ForumThread createThread() {
User bob = User.builder().firstName("Bob").lastName("the Builder").emailAddress("bob@building.com").build(); User bob = User.builder().firstName("Bob").lastName("the Builder").emailAddress("bob@building.com").build();
ForumPost post = new ForumPost(); post = new ForumPost();
post.setPostedBy(bob); post.setPostedBy(bob);
ForumThread groupForumThread = new ForumThread(); ForumThread groupForumThread = new ForumThread();
groupForumThread.addPost(post); groupForumThread.addPost(post);