Skip to content
This repository has been archived by the owner on Feb 13, 2023. It is now read-only.

Remove and edit producs groups #40

Merged
merged 5 commits into from
Mar 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ch.wisv.payments.admin.committees;

import ch.wisv.payments.model.Committee;
import ch.wisv.payments.model.CommitteeEnum;

import java.util.List;

Expand All @@ -9,4 +10,8 @@ public interface CommitteeService {
List<Committee> getAllCommittees();

void addCommittee(Committee committee);

Committee getCommitteeById(int committeeId);

Committee getCommittee(CommitteeEnum committeeEnum, int year);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package ch.wisv.payments.admin.committees;

import ch.wisv.payments.exception.CommitteeNotFoundException;
import ch.wisv.payments.model.Committee;
import ch.wisv.payments.model.CommitteeEnum;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.stereotype.Service;
Expand All @@ -25,8 +27,19 @@ public List<Committee> getAllCommittees() {
@Override
public void addCommittee(Committee committee) {
if (committeeRepository.findOneByNameAndYear(committee.getName(), committee.getYear()).isPresent()) {
throw new DuplicateKeyException("Commitee already exists!");
throw new DuplicateKeyException("Committee already exists!");
}
committeeRepository.save(committee);
}

@Override
public Committee getCommitteeById(int committeeId) {
return committeeRepository.findOne(committeeId);
}

@Override
public Committee getCommittee(CommitteeEnum committeeEnum, int year) {
return committeeRepository.findOneByNameAndYear(committeeEnum, year)
.orElseThrow(CommitteeNotFoundException::new);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import ch.wisv.payments.admin.products.request.ProductGroupRequest;
import ch.wisv.payments.admin.products.request.ProductRequest;
import ch.wisv.payments.model.Product;
import ch.wisv.payments.model.ProductGroup;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
Expand Down Expand Up @@ -43,7 +44,7 @@ public String productEdit(@PathVariable Integer productId, Model model) {
Product product = productService.getProductById(productId);
ProductRequest productRequest = new ProductRequest();

productRequest.setProductId(product.getId());
productRequest.setId(product.getId());
productRequest.setName(product.getName());
productRequest.setDescription(product.getDescription());
productRequest.setPrice(product.getPrice());
Expand All @@ -58,40 +59,81 @@ public String productEdit(@PathVariable Integer productId, Model model) {

model.addAttribute("product", productRequest);

return "edit";
return "editProduct";
}

@PostMapping(value = "/add")
public String addProduct(@ModelAttribute @Validated ProductRequest productRequest) {
public String addProduct(@ModelAttribute @Validated ProductRequest productRequest, RedirectAttributes redirectAttributes) {
productService.addProduct(productRequest);

redirectAttributes.addFlashAttribute("message", productRequest.getName() + " successfully added.");
return "redirect:/products";
}

@PostMapping(value = "/edit")
public String editProduct(@ModelAttribute @Validated ProductRequest productRequest, RedirectAttributes redirectAttributes) {
productService.editProduct(productRequest);

redirectAttributes.addFlashAttribute("message", productRequest.getName() + " successfully updated!");

redirectAttributes.addFlashAttribute("message", productRequest.getName() + " successfully updated.");
return "redirect:/products";
}

@PostMapping(value = "/delete/{productId}")
public String deleteProduct(@PathVariable int productId, RedirectAttributes redirectAttributes) {
try {
productService.deleteProduct(productId);
} catch (RuntimeException e) {
redirectAttributes.addFlashAttribute("error", e.getMessage());
}
productService.deleteProduct(productId);
redirectAttributes.addFlashAttribute("message", "Product successfully removed.");

return "redirect:/products";
}

@PostMapping(value = "/addGroup")
public String addProductGroup(@ModelAttribute @Validated ProductGroupRequest productGroupRequest) {
@PostMapping(value = "/group/add")
public String addProductGroup(@ModelAttribute @Validated ProductGroupRequest productGroupRequest, RedirectAttributes redirectAttributes) {
productService.addProductGroup(productGroupRequest);

redirectAttributes.addFlashAttribute("message", "Product group " + productGroupRequest.getName() + " successfully added.");

return "redirect:/products";
}

@GetMapping(value = "/group/edit/{productGroupId}")
public String productGroupEdit(@PathVariable Integer productGroupId, Model model) {
model.addAttribute("committees", committeeService.getAllCommittees());

ProductGroup productGroup = productService.getProductGroupById(productGroupId);
ProductGroupRequest productGroupRequest = new ProductGroupRequest();

productGroupRequest.setId(productGroup.getId());
productGroupRequest.setName(productGroup.getName());
productGroupRequest.setCommitteeId(productGroup.getCommittee().getId());
productGroupRequest.setDescription(productGroup.getDescription());
productGroupRequest.setGroupLimit(productGroup.getGroupLimit());

model.addAttribute("productGroup", productGroupRequest);

return "editProductGroup";
}

@PostMapping(value = "/group/edit")
public String editProductGroup(@ModelAttribute @Validated ProductGroupRequest productGroupRequest, RedirectAttributes redirectAttributes) {
productService.editProductGroup(productGroupRequest);

redirectAttributes.addFlashAttribute("message", productGroupRequest.getName() + " successfully updated.");

return "redirect:/products";
}

@PostMapping(value = "/group/delete/{productGroupId}")
public String deleteProductGroup(@PathVariable int productGroupId, RedirectAttributes redirectAttributes) {
productService.deleteProductGroup(productGroupId);
redirectAttributes.addFlashAttribute("message", "Product group successfully removed.");

return "redirect:/products";
}

@ExceptionHandler(RuntimeException.class)
public String formErrorHandler(RuntimeException e, RedirectAttributes redirectAttributes) {
redirectAttributes.addFlashAttribute("error", e.getMessage());

return "redirect:/products";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,10 @@ public interface ProductService {
void addProductGroup(ProductGroupRequest productGroupRequest);

void addProductToGroup(Product product, ProductGroup productGroup);

void deleteProductGroup(int productGroupId);

ProductGroup getProductGroupById(Integer productGroupId);

void editProductGroup(ProductGroupRequest productGroupRequest);
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package ch.wisv.payments.admin.products;

import ch.wisv.payments.admin.committees.CommitteeRepository;
import ch.wisv.payments.admin.committees.CommitteeService;
import ch.wisv.payments.admin.products.request.ProductGroupRequest;
import ch.wisv.payments.admin.products.request.ProductRequest;
import ch.wisv.payments.exception.CommmitteeNotFoundException;
import ch.wisv.payments.exception.ProductGroupInUseException;
import ch.wisv.payments.exception.ProductInUseException;
import ch.wisv.payments.model.*;
import ch.wisv.payments.rest.OrderService;
import ch.wisv.payments.rest.repository.ProductGroupRepository;
import ch.wisv.payments.rest.repository.ProductRepository;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;

import java.util.List;
Expand All @@ -20,14 +19,13 @@ public class ProductServiceImpl implements ProductService {

private ProductRepository productRepository;
private ProductGroupRepository productGroupRepository;
private CommitteeRepository committeeRepository;
private CommitteeService committeeService;
private OrderService orderService;

@Autowired
public ProductServiceImpl(ProductRepository productRepository, ProductGroupRepository productGroupRepository, CommitteeRepository committeeRepository, OrderService orderService) {
public ProductServiceImpl(ProductRepository productRepository, ProductGroupRepository productGroupRepository, CommitteeService committeeService, OrderService orderService) {
this.productRepository = productRepository;
this.productGroupRepository = productGroupRepository;
this.committeeRepository = committeeRepository;
this.committeeService = committeeService;
this.orderService = orderService;
}

Expand All @@ -38,7 +36,7 @@ public List<Product> getAllProducts() {

@Override
public void addProduct(ProductRequest productRequest) {
Committee committee = committeeRepository.findOne(productRequest.getCommitteeId());
Committee committee = committeeService.getCommitteeById(productRequest.getCommitteeId());
Product product = new Product(committee,
productRequest.getName(),
productRequest.getDescription(),
Expand All @@ -56,7 +54,7 @@ public void addProduct(ProductRequest productRequest) {

@Override
public void addProductGroup(ProductGroupRequest productGroupRequest) {
Committee committee = committeeRepository.findOne(productGroupRequest.getCommitteeId());
Committee committee = committeeService.getCommitteeById(productGroupRequest.getCommitteeId());
ProductGroup group = new ProductGroup(productGroupRequest.getName(),
productGroupRequest.getDescription(), productGroupRequest.getGroupLimit(), committee);

Expand All @@ -79,9 +77,9 @@ public void addProductToGroup(Product product, ProductGroup productGroup) {

@Override
public void editProduct(ProductRequest productRequest) {
if (productRequest.getProductId() != 0) {
Committee committee = committeeRepository.findOne(productRequest.getCommitteeId());
Product product = productRepository.findOne(productRequest.getProductId());
if (productRequest.getId() != 0) {
Committee committee = committeeService.getCommitteeById(productRequest.getCommitteeId());
Product product = productRepository.findOne(productRequest.getId());

product.setName(productRequest.getName());
product.setDescription(productRequest.getDescription());
Expand All @@ -102,15 +100,25 @@ public void editProduct(ProductRequest productRequest) {
}
}

@Override
public void deleteProduct(int productId) {
List<Order> orders = orderService.getOrdersByProductId(productId);

if (orders.size() > 0) {
throw new ProductInUseException("Products are already ordered");
} else {
productRepository.delete(productId);
}
}

@Override
public Product getProductById(Integer productId) {
return productRepository.findOne(productId);
}

@Override
public Set<Product> getProductByCommittee(CommitteeEnum committeeEnum, int year) {
Committee committee = committeeRepository.findOneByNameAndYear(committeeEnum, year)
.orElseThrow(CommmitteeNotFoundException::new);
Committee committee = committeeService.getCommittee(committeeEnum, year);

return productRepository.findByCommittee(committee);
}
Expand All @@ -130,13 +138,32 @@ public boolean isProductAvailable(Integer productId) {
}

@Override
public void deleteProduct(int productId) {
List<Order> orders = orderService.getOrdersByProductId(productId);
public ProductGroup getProductGroupById(Integer productGroupId) {
return productGroupRepository.findOne(productGroupId);
}

if (orders.size() > 0) {
throw new ProductInUseException("Products are already ordered");
@Override
public void editProductGroup(ProductGroupRequest productGroupRequest) {
if (productGroupRequest.getId() != 0) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this if?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was also in the editProduct() method, could you shed some light on this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, probably because the id's are int, which defaults to 0 instead of null

Committee committee = committeeService.getCommitteeById(productGroupRequest.getCommitteeId());
ProductGroup productGroup = productGroupRepository.findOne(productGroupRequest.getId());

productGroup.setCommittee(committee);
productGroup.setName(productGroupRequest.getName());
productGroup.setDescription(productGroupRequest.getDescription());
productGroup.setGroupLimit(productGroupRequest.getGroupLimit());

productGroupRepository.saveAndFlush(productGroup);
}
}

@Override
public void deleteProductGroup(int productGroupId) {
ProductGroup productGroup = productGroupRepository.findOne(productGroupId);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Servicecall here too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do we want to make a new ProductGroupService and ProductGroupServiceImpl? Either that or leave it here.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can put that in one service. Once we need more functionality that requires a lot more methods, we'll separate them.

if (!productGroup.getProducts().isEmpty()) {
throw new ProductGroupInUseException("Product group must be empty");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really the case? What would break if you remove a group with products in it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had previously implemented it in a way where products were removed from the group when the group was removed, but this may be an action where we don't want it to be very easy to perform. I'm also not quite sure which way to go, it made sense to me that since there is a limit involved, you don't want it to be this easy, just like removing a product that is already ordered.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, then perhaps add a section to the editGroup page where you can remove products currently in the group?

Copy link
Contributor Author

@martijnjanssen martijnjanssen Mar 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also add the removal of tickets from a group as a hoothub issue, since this is not very high prioroty. It is also not relevant to editing the product group, it would make more sense to have it in a different page. /products/group/editproductsor something similar.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave it like this for now, and create a HootHub issue to implement this nicely.

} else {
productRepository.delete(productId);
productGroupRepository.delete(productGroupId);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class ProductRequest {

@Getter
@Setter
int productId;
int id;
@Setter
@Getter
int committeeId;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package ch.wisv.payments.exception;

public class CommitteeNotFoundException extends RuntimeException {
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package ch.wisv.payments.exception;

public class EmptyOrderException extends RuntimeException {

public EmptyOrderException(String s) {
super(s);
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package ch.wisv.payments.exception;

public class ProductGroupInUseException extends RuntimeException {
public ProductGroupInUseException(String s) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor is already in RuntimeException

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You unfortunately have to do this to make use of the super(...) constructor though.

super(s);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@ <h3 class="panel-title">Edit</h3>
</div>
<div class="panel-body col-lg-12">
<!--/*@thymesVar id="product" type="ch.wisv.payments.admin.products.request.ProductRequest"*/-->
<form class="form-horizontal" th:action="@{/products/edit}" th:object="${product}"
method="post">
<input type="hidden" th:field="*{productId}">
<form th:action="@{/products/edit}" th:object="${product}" method="post">
<input type="hidden" th:field="*{id}">
<div class="form-group">
<label for="committeeName">Committee</label>
<select class="form-control" th:field="*{committeeId}" id="committeeName">
Expand Down Expand Up @@ -110,7 +109,7 @@ <h3 class="panel-title">Edit</h3>
<input type="text" class="form-control" th:field="*{returnURL}" id="ReturnURL"
placeholder="Return URL">
</div>
<button type="submit" class="btn btn-default">Edit product</button>
<button type="submit" class="btn btn-success">Save product</button>
</form>
</div>
</div>
Expand Down
Loading