-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Story/vspc 190 A user should be able to define the order spaces appear in the site menu of the public page #277
base: develop
Are you sure you want to change the base?
Conversation
…y user instead of drag and drop
Make it so, Jenkins. |
# Conflicts: # vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/ExhibitionManager.java
vspace/src/main/java/edu/asu/diging/vspace/core/model/ExhibitionSpaceOrderMode.java
Show resolved
Hide resolved
return value; | ||
} | ||
|
||
public static List<String> getAllValues() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can probably just get rid of the values, and simply pass ALPHABETICAL
or CREATION_DATE
as request parameter. Then you can use the valueOf
method to get an enum.
vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/ExhibitionSpaceOrderUtility.java
Show resolved
Hide resolved
vspace/src/main/java/edu/asu/diging/vspace/core/services/impl/ExhibitionSpaceOrderUtility.java
Show resolved
Hide resolved
* @param publishedSpaces | ||
* @return list of {@link ISpace} | ||
*/ | ||
private List<ISpace> sortSpacesOnCreationDate(List<ISpace> publishedSpaces){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
attributes.addFlashAttribute("alertType", "danger"); | ||
attributes.addFlashAttribute("message", "Custom order could not be updated. The title field is empty."); | ||
attributes.addFlashAttribute("showAlert", "true"); | ||
return "redirect:/staff/space/order/"+spacesCustomOrderId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as before, data should not be lost.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
@Autowired | ||
private ISpacesCustomOrderManager spacesCustomOrderManager; | ||
|
||
@RequestMapping(value = "/staff/space/order/{customOrderId}/edit/info", method = RequestMethod.POST) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and then this url can just be /staff/space/order/{customOrderId}
} | ||
|
||
@RequestMapping(value = "/staff/space/order/{customOrderId}/edit/spaceorders", method = RequestMethod.POST) | ||
public String saveOrder( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there are reason, name/description and order are not stored together in one call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right now, only the order can be changed on the edit page, and on the same page, there is a separate option to edit the name/description, which opens a popup to edit and save only the name/description.
Should we keep it all together (name, description and order) in one page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I don't see a reason not to
*/ | ||
|
||
@Controller | ||
public class StaffSpaceCustomOrderController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name should make clear what this controller is for
import edu.asu.diging.vspace.core.services.ISpacesCustomOrderManager; | ||
|
||
@Controller | ||
public class UpdateStaffSpaceOrderModeController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this called ...StaffSpace...
not just ...Space...
? There is not equivalent for the public page, right? This controller does not need to be contrasted with another one for the public site?
.getSpacesCustomOrder(); | ||
if(spaceCustomOrder!=null && spaceCustomOrder.getId().equals(id)) { | ||
exhibition.setSpacesCustomOrder(null); | ||
exhibitionManager.storeExhibition((Exhibition)exhibition); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
attributes.addFlashAttribute("alertType", "danger"); | ||
attributes.addFlashAttribute("message", "Custom order could not be saved. Please enter the name of the order."); | ||
attributes.addFlashAttribute("showAlert", "true"); | ||
return "redirect:/staff/space/order/add"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
attributes.addFlashAttribute("alertType", "danger"); | ||
attributes.addFlashAttribute("message", "Custom order could not be updated. The title field is empty."); | ||
attributes.addFlashAttribute("showAlert", "true"); | ||
return "redirect:/staff/space/order/"+spacesCustomOrderId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
} | ||
|
||
@RequestMapping(value = "/staff/space/order", method = RequestMethod.GET) | ||
public String displayCustomOrders(Model model) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
} | ||
|
||
@RequestMapping(value = "/staff/space/order/setExhibitionCustomOrder", method = RequestMethod.POST) | ||
public String setExhibitionSpacesCustomOrder(Model model, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be in the same class as the one to show the page for a specific order, but url should be /staff/space/order/{orderId}
.
Guidelines for Pull Requests
If you haven't yet read our code review guidelines, please do so, You can find them here.
Please confirm the following by adding an x for each item (turn
[ ]
into[x]
).Please provide a brief description of your ticket
(you can copy the ticket if it hasn't changed)
...Put description here...
Right now, on the public site, the menu simply lists all published spaces. The user has no control over the order. For this ticket two things should be implemented:
by default the spaces should be ordered alphabetically by name
a user should be able to switch between alphabetically ordered, ordered by creation date, and a custom order.
For the custom order, there should be a page (on the staff side) that lists all spaces and the user can simply arrange them in the order they should be shown.
Anything else the reviewer needs to know?
User can choose if they have to arrange spaces alphabetically or through creation date or a custom order in spaces on the staff site.
They can create custom orders in the custom order page on the staff site. Alphabetical is default, if user chooses custom but does not select an order it will fall back to alphabetical order.