Skip to content

Commit

Permalink
SAK-41391: Polls > in an Oracle environment poll options are not disp…
Browse files Browse the repository at this point in the history
…layed in the order they're created (sakaiproject#6593)
  • Loading branch information
bjones86 authored Mar 19, 2019
1 parent d91cbae commit 3ea1250
Show file tree
Hide file tree
Showing 18 changed files with 229 additions and 87 deletions.
35 changes: 18 additions & 17 deletions polls/api/src/java/org/sakaiproject/poll/hbm/Option.hbm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
<!-- create the primary key id, using native is typically the best way
to do this -->
<id name="optionId" type="long" column="OPTION_ID">
<generator class="native">
<param name="sequence">POLL_OPTION_ID_SEQ</param>
</generator>
<generator class="native">
<param name="sequence">POLL_OPTION_ID_SEQ</param>
</generator>
</id>
<!-- The remaining properties should just match the properties
of your value object.
Expand All @@ -20,18 +20,19 @@
Column names are optional but people often specify them. -->

<property name="pollId" type="long" not-null="true">
<column name="OPTION_POLL_ID"/>
</property>
<property name="optionText" type="materialized_clob" length="255" not-null="true">
<column name="OPTION_TEXT"/>
</property>
<property name="Uuid" type="string" length="255" not-null="true">
<column name="OPTION_UUID" />
</property>
<property name="deleted" type="java.lang.Boolean">
<column name="DELETED" />
</property>
</class>


<column name="OPTION_POLL_ID"/>
</property>
<property name="text" type="materialized_clob" length="255" not-null="true">
<column name="OPTION_TEXT"/>
</property>
<property name="Uuid" type="string" length="255" not-null="true">
<column name="OPTION_UUID" />
</property>
<property name="deleted" type="java.lang.Boolean">
<column name="DELETED" />
</property>
<property name="optionOrder" type="int" not-null="true">
<column name="OPTION_ORDER" />
</property>
</class>
</hibernate-mapping>
13 changes: 1 addition & 12 deletions polls/api/src/java/org/sakaiproject/poll/model/Option.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,15 @@ public class Option {
private String status;
private String uuid;
private Boolean deleted = Boolean.FALSE;
private Integer optionOrder;

public Option() {}

public Option(Long oId) {
this.optionId = oId;
}

public void setOptionText(String option) {
text = option;
}

public String getOptionText() {
return text;
}

public String getId() {
return optionId+"";
}

public void setId(Long id) {
this.optionId = optionId;
}
}
4 changes: 2 additions & 2 deletions polls/api/src/java/org/sakaiproject/poll/util/PollUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static Element optionToXml(Option option, Document doc, Stack<Element> st

element.setAttribute("id", option.getUuid());
element.setAttribute("optionid", option.getOptionId().toString());
element.setAttribute("title", option.getOptionText());
element.setAttribute("title", option.getText());
element.setAttribute("deleted", option.getDeleted().toString());
stack.pop();

Expand All @@ -69,7 +69,7 @@ public static Option xmlToOption(Element element) {
//LOG THIS
}
}
option.setOptionText(element.getAttribute(TEXT));
option.setText(element.getAttribute(TEXT));
option.setDeleted(Boolean.parseBoolean(element.getAttribute(DELETED)));
return option;
}
Expand Down
10 changes: 9 additions & 1 deletion polls/impl/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-collections4</artifactId>
</dependency>
<dependency>
<groupId>org.dom4j</groupId>
<artifactId>dom4j</artifactId>
Expand Down Expand Up @@ -126,6 +130,10 @@
<artifactId>oro</artifactId>
<version>2.0.8</version>
</dependency>
<dependency>
<groupId>org.quartz-scheduler</groupId>
<artifactId>quartz</artifactId>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* Copyright (c) 2003-2019 The Apereo Foundation
*
* Licensed under the Educational Community License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://opensource.org/licenses/ecl2
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.sakaiproject.poll.logic.impl;

import java.util.Comparator;
import java.util.List;

import lombok.Getter;
import lombok.Setter;
import lombok.extern.slf4j.Slf4j;

import org.apache.commons.collections4.CollectionUtils;

import org.quartz.DisallowConcurrentExecution;
import org.quartz.Job;
import org.quartz.JobExecutionContext;
import org.quartz.JobExecutionException;

import org.sakaiproject.authz.api.SecurityAdvisor;
import org.sakaiproject.authz.api.SecurityService;
import org.sakaiproject.poll.logic.PollListManager;
import org.sakaiproject.poll.model.Option;
import org.sakaiproject.poll.model.Poll;
import org.sakaiproject.site.api.Site;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.site.api.SiteService.SelectionType;
import org.sakaiproject.site.api.SiteService.SortType;

/**
* This Quartz job is responsible for back-filling the 'order' attribute of all existing Poll options.
* It will iterate over all sites in the system, ordered by creation date. It will then request any
* existing Polls for the site, starting with the newest and working backwards, and back-fill the
* 'order' attribute of each poll's options based on the option's original ID value. If the options
* are not null, the job will abort to avoid re-setting already existing orders (in case the job is
* accidentally run more than once).
*
* @author Brian Jones ([email protected])
*/
@Slf4j
@DisallowConcurrentExecution
public class PollOrderOptionBackFillJob implements Job
{
// APIs
@Getter @Setter private SiteService siteService;
@Getter @Setter private SecurityService securityService;
@Getter @Setter private PollListManager pollService;

private static final SecurityAdvisor YES_MAN = (String userId, String function, String reference) -> SecurityAdvisor.SecurityAdvice.ALLOWED;

/**
* This is the method that is fired when the job is 'triggered'.
*
* @param jobExecutionContext - the context of the job execution
* @throws JobExecutionException
*/
@Override
public void execute( JobExecutionContext jobExecutionContext ) throws JobExecutionException
{
log.info( "Attempting to back-fill all existing Poll option orders..." );
int modifiedCount = 0;

// Iterate over sites in the system, ordered by creation date...
List<Site> sites = siteService.getSites( SelectionType.ANY, null, null, null, SortType.CREATED_ON_DESC, null );
if( !CollectionUtils.isEmpty( sites ) )
{
for( Site site : sites )
{
String siteID = site.getId();
List<Poll> pollsForSite = pollService.findAllPolls( siteID );
if( !CollectionUtils.isEmpty( pollsForSite ) )
{
// Iterate over the polls for the site...
for( Poll poll : pollsForSite )
{
try
{
// Iterate over Options in the poll...
securityService.pushAdvisor( YES_MAN );
List<Option> pollOptions = pollService.getOptionsForPoll( poll );
if( !CollectionUtils.isEmpty( pollOptions ) )
{
// Check if any options have a null order
boolean hasNullOptionOrder = false;
for( Option option : pollOptions )
{
if( option.getOptionOrder() == null )
{
hasNullOptionOrder = true;
break;
}
}

// If any of the option's order is null, we need to back-fill them
if( hasNullOptionOrder )
{
log.info( "Poll ID {} has options with null order, processing...", poll.getId() );

// Order the list by ID
pollOptions.sort( Comparator.comparingLong( Option::getOptionId ) );

// Iterate over the list
for( int i = 0; i < pollOptions.size(); i++ )
{
// Add order based on ID
Option option = pollOptions.get( i );
option.setOptionOrder(i);
pollService.saveOption( option );
modifiedCount++;
log.info( "Option {} ---> new order == {}", option.getId(), i );
}
}
}
}
catch( Exception ex )
{
log.error( "Unexcepted exception", ex );
}
finally
{
securityService.popAdvisor( YES_MAN );
}
}
}
}
}

log.info( "Processing finished, modified {} poll options", modifiedCount );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@

import lombok.extern.slf4j.Slf4j;
import lombok.Data;

import org.springframework.dao.DataAccessException;

import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
Expand Down Expand Up @@ -280,7 +282,7 @@ public List<Option> getOptionsForPoll(Long pollId) {
}
Search search = new Search();
search.addRestriction(new Restriction("pollId", pollId));
search.addOrder(new Order("optionId"));
search.addOrder(new Order("optionOrder"));
List<Option> optionList = dao.findBySearch(Option.class, search);
return optionList;
}
Expand Down Expand Up @@ -569,7 +571,7 @@ public void transferCopyEntities(String fromContext, String toContext, List reso
while (fromOptions.hasNext()){
Option fromOption = (Option) fromOptions.next();
Option toOption = (Option) new Option();
toOption.setOptionText(fromOption.getOptionText());
toOption.setText(fromOption.getText());
toOption.setStatus(fromOption.getStatus());
toOption.setPollId(toPoll.getPollId());
toOption.setDeleted(fromOption.getDeleted());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,11 @@ public void testDeletePoll() {

Option option1 = new Option();
option1.setPollId(poll1.getPollId());
option1.setOptionText("asdgasd");
option1.setText("asdgasd");

Option option2 = new Option();
option2.setPollId(poll1.getPollId());
option2.setOptionText("zsdbsdfb");
option2.setText("zsdbsdfb");

pollListManager.saveOption(option2);
pollListManager.saveOption(option1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,12 @@ public void preloadTestData(GenericDao dao) {

//add some options
Option option1 = new Option();
option1.setOptionText("Option 1");
option1.setText("Option 1");
option1.setPollId(poll1.getPollId());
dao.save(option1);

Option option2 = new Option();
option2.setOptionText("Option 2");
option2.setText("Option 2");
option2.setPollId(poll1.getPollId());
dao.save(option2);

Expand Down
23 changes: 23 additions & 0 deletions polls/impl/src/webapp/WEB-INF/components.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,29 @@
<property name="emailTemplates" ref="org.sakaiproject.poll.emailtemplates.List" />
</bean>

<!-- Job to back-fill Poll options' order -->
<bean id="pollOrderOptionBackFillJob" class="org.sakaiproject.poll.logic.impl.PollOrderOptionBackFillJob">
<property name="siteService" ref="org.sakaiproject.site.api.SiteService" />
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService" />
<property name="pollService" ref="org.sakaiproject.poll.logic.PollListManager" />
</bean>
<bean id="org.sakaiproject.api.app.scheduler.JobBeanWrapper.pollOrderOptionBackFillJob"
class="org.sakaiproject.component.app.scheduler.jobs.SpringJobBeanWrapper"
init-method="init">

<property name="beanId">
<value>pollOrderOptionBackFillJob</value>
</property>

<property name="jobName">
<value>Backfill all existing poll options' order based on their original ID</value>
</property>

<property name="schedulerManager">
<ref bean="org.sakaiproject.api.app.scheduler.SchedulerManager" />
</property>
</bean>

<!-- Setup email templates -->
<bean id="org.sakaiproject.poll.emailtemplates.List" class="java.util.ArrayList">
<constructor-arg>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public String createEntity(EntityReference ref, Object entity, Map<String, Objec
throw new IllegalArgumentException("Poll ID must be set to create an option");
}
// check minimum settings
if (option.getOptionText() == null) {
if (option.getText() == null) {
throw new IllegalArgumentException("Poll Option text must be set to create an option");
}
checkOptionPermission(userReference, option);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ public String createOptionEntity(EntityReference ref,
"Poll ID must be set to create an option");
}
// check minimum settings
if (option.getOptionText() == null) {
if (option.getText() == null) {
throw new IllegalArgumentException(
"Poll Option text must be set to create an option");
}
Expand Down
Loading

0 comments on commit 3ea1250

Please sign in to comment.