Centralized ticket editing permission controls

3 files changed, 78 insertions, 84 deletions

diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
index b3c102d9..b5fe0ae5 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.html
@@ -38,8 +38,7 @@
 				</div>

 				</div>

 			</td></tr>

-			<tr><th><wicket:message key="gb.type"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="type"></select></td></tr>

-			<tr><th><wicket:message key="gb.status"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="status"></select></td></tr>

+			<tr wicket:id="status"></tr>

 			<tr wicket:id="responsible"></tr>

 			<tr wicket:id="milestone"></tr>

 			<tr wicket:id="mergeto"></tr>

@@ -56,6 +55,10 @@
 </div>

 </body>

 

+<wicket:fragment wicket:id="statusFragment">

+	<tr><th><wicket:message key="gb.status"></wicket:message><span style="color:red;">*</span></th><td class="edit"><select class="input-large" wicket:id="status"></select></td></tr>

+</wicket:fragment>

+

 <wicket:fragment wicket:id="responsibleFragment">

 	<th><wicket:message key="gb.responsible"></wicket:message></th><td class="edit"><select class="input-large" wicket:id="responsible"></select></td>

 </wicket:fragment>

diff --git a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
index ac7ff97e..5fa31975 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditTicketPage.java
@@ -88,11 +88,6 @@ public class EditTicketPage extends RepositoryPage {
 			currentUser = UserModel.ANONYMOUS;

 		}

 

-		if (!currentUser.isAuthenticated || !app().tickets().isAcceptingTicketUpdates(getRepositoryModel())) {

-			// tickets prohibited

-			setResponsePage(TicketsPage.class, WicketUtils.newRepositoryParameter(repositoryName));

-		}

-

 		long ticketId = 0L;

 		try {

 			String h = WicketUtils.getObject(params);

@@ -102,8 +97,10 @@ public class EditTicketPage extends RepositoryPage {
 		}

 

 		TicketModel ticket = app().tickets().getTicket(getRepositoryModel(), ticketId);

-		if (ticket == null) {

-			setResponsePage(TicketsPage.class, WicketUtils.newRepositoryParameter(repositoryName));

+		if (ticket == null

+				|| !currentUser.canEdit(ticket, getRepositoryModel())

+				|| !app().tickets().isAcceptingTicketUpdates(getRepositoryModel())) {

+			setResponsePage(TicketsPage.class, WicketUtils.newObjectParameter(repositoryName, "" + ticketId));

 		}

 

 		typeModel = Model.of(ticket.type);

@@ -223,18 +220,6 @@ public class EditTicketPage extends RepositoryPage {
 		}

 		form.add(new DropDownChoice<TicketModel.Type>("type", typeModel, typeChoices));

 

-		List<Status> statusChoices;

-		if (ticket.isClosed()) {

-			statusChoices = Arrays.asList(ticket.status, Status.Open);

-		} else if (ticket.isProposal()) {

-			statusChoices = Arrays.asList(TicketModel.Status.proposalWorkflow);

-		} else if (ticket.isBug()) {

-			statusChoices = Arrays.asList(TicketModel.Status.bugWorkflow);

-		} else {

-			statusChoices = Arrays.asList(TicketModel.Status.requestWorkflow);

-		}

-		form.add(new DropDownChoice<TicketModel.Status>("status", statusModel, statusChoices));

-

 		form.add(new TextField<String>("title", titleModel));

 		form.add(new TextField<String>("topic", topicModel));

 

@@ -249,78 +234,85 @@ public class EditTicketPage extends RepositoryPage {
 		descriptionEditor.setText(ticket.body);

 		form.add(descriptionEditor);

 

-		if (currentUser != null && currentUser.isAuthenticated && currentUser.canPush(getRepositoryModel())) {

-			// responsible

-			Set<String> userlist = new TreeSet<String>(ticket.getParticipants());

+		// status

+		List<Status> statusChoices;

+		if (ticket.isClosed()) {

+			statusChoices = Arrays.asList(ticket.status, Status.Open);

+		} else if (ticket.isProposal()) {

+			statusChoices = Arrays.asList(TicketModel.Status.proposalWorkflow);

+		} else if (ticket.isBug()) {

+			statusChoices = Arrays.asList(TicketModel.Status.bugWorkflow);

+		} else {

+			statusChoices = Arrays.asList(TicketModel.Status.requestWorkflow);

+		}

+		Fragment status = new Fragment("status", "statusFragment", this);

+		status.add(new DropDownChoice<TicketModel.Status>("status", statusModel, statusChoices));

+		form.add(status);

 

-			for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) {

-				if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) {

-					userlist.add(rp.registrant);

-				}

-			}

+		// responsible

+		Set<String> userlist = new TreeSet<String>(ticket.getParticipants());

 

-			List<TicketResponsible> responsibles = new ArrayList<TicketResponsible>();

-			for (String username : userlist) {

-				UserModel user = app().users().getUserModel(username);

-				if (user != null) {

-					TicketResponsible responsible = new TicketResponsible(user);

-					responsibles.add(responsible);

-					if (user.username.equals(ticket.responsible)) {

-						responsibleModel.setObject(responsible);

-					}

-				}

+		for (RegistrantAccessPermission rp : app().repositories().getUserAccessPermissions(getRepositoryModel())) {

+			if (rp.permission.atLeast(AccessPermission.PUSH) && !rp.isTeam()) {

+				userlist.add(rp.registrant);

 			}

-			Collections.sort(responsibles);

-			responsibles.add(new TicketResponsible(NIL, "", ""));

-			Fragment responsible = new Fragment("responsible", "responsibleFragment", this);

-			responsible.add(new DropDownChoice<TicketResponsible>("responsible", responsibleModel, responsibles));

-			form.add(responsible.setVisible(!responsibles.isEmpty()));

-

-			// milestone

-			List<TicketMilestone> milestones = app().tickets().getMilestones(getRepositoryModel(), Status.Open);

-			for (TicketMilestone milestone : milestones) {

-				if (milestone.name.equals(ticket.milestone)) {

-					milestoneModel.setObject(milestone);

-					break;

+		}

+

+		List<TicketResponsible> responsibles = new ArrayList<TicketResponsible>();

+		for (String username : userlist) {

+			UserModel user = app().users().getUserModel(username);

+			if (user != null) {

+				TicketResponsible responsible = new TicketResponsible(user);

+				responsibles.add(responsible);

+				if (user.username.equals(ticket.responsible)) {

+					responsibleModel.setObject(responsible);

 				}

 			}

-			if (milestoneModel.getObject() == null && !StringUtils.isEmpty(ticket.milestone)) {

-				// ensure that this unrecognized milestone is listed

-				// so that we get the <nil> selection.

-				TicketMilestone tms = new TicketMilestone(ticket.milestone);

-				milestones.add(tms);

-				milestoneModel.setObject(tms);

-			}

-			if (!milestones.isEmpty()) {

-				milestones.add(new TicketMilestone(NIL));

+		}

+		Collections.sort(responsibles);

+		responsibles.add(new TicketResponsible(NIL, "", ""));

+		Fragment responsible = new Fragment("responsible", "responsibleFragment", this);

+		responsible.add(new DropDownChoice<TicketResponsible>("responsible", responsibleModel, responsibles));

+		form.add(responsible.setVisible(!responsibles.isEmpty()));

+

+		// milestone

+		List<TicketMilestone> milestones = app().tickets().getMilestones(getRepositoryModel(), Status.Open);

+		for (TicketMilestone milestone : milestones) {

+			if (milestone.name.equals(ticket.milestone)) {

+				milestoneModel.setObject(milestone);

+				break;

 			}

+		}

+		if (milestoneModel.getObject() == null && !StringUtils.isEmpty(ticket.milestone)) {

+			// ensure that this unrecognized milestone is listed

+			// so that we get the <nil> selection.

+			TicketMilestone tms = new TicketMilestone(ticket.milestone);

+			milestones.add(tms);

+			milestoneModel.setObject(tms);

+		}

+		if (!milestones.isEmpty()) {

+			milestones.add(new TicketMilestone(NIL));

+		}

 

-			Fragment milestone = new Fragment("milestone", "milestoneFragment", this);

+		Fragment milestone = new Fragment("milestone", "milestoneFragment", this);

 

-			milestone.add(new DropDownChoice<TicketMilestone>("milestone", milestoneModel, milestones));

-			form.add(milestone.setVisible(!milestones.isEmpty()));

+		milestone.add(new DropDownChoice<TicketMilestone>("milestone", milestoneModel, milestones));

+		form.add(milestone.setVisible(!milestones.isEmpty()));

 

-			// mergeTo (integration branch)

-			List<String> branches = new ArrayList<String>();

-			for (String branch : getRepositoryModel().getLocalBranches()) {

-				// exclude ticket branches

-				if (!branch.startsWith(Constants.R_TICKET)) {

-					branches.add(Repository.shortenRefName(branch));

-				}

+		// mergeTo (integration branch)

+		List<String> branches = new ArrayList<String>();

+		for (String branch : getRepositoryModel().getLocalBranches()) {

+			// exclude ticket branches

+			if (!branch.startsWith(Constants.R_TICKET)) {

+				branches.add(Repository.shortenRefName(branch));

 			}

-			branches.remove(Repository.shortenRefName(getRepositoryModel().HEAD));

-			branches.add(0, Repository.shortenRefName(getRepositoryModel().HEAD));

-

-			Fragment mergeto = new Fragment("mergeto", "mergeToFragment", this);

-			mergeto.add(new DropDownChoice<String>("mergeto", mergeToModel, branches));

-			form.add(mergeto.setVisible(!branches.isEmpty()));

-

-		} else {

-			// user does not have permission to assign milestone or responsible

-			form.add(new Label("responsible").setVisible(false));

-			form.add(new Label("milestone").setVisible(false));

-			form.add(new Label("mergeto").setVisible(false));

 		}

+		branches.remove(Repository.shortenRefName(getRepositoryModel().HEAD));

+		branches.add(0, Repository.shortenRefName(getRepositoryModel().HEAD));

+

+		Fragment mergeto = new Fragment("mergeto", "mergeToFragment", this);

+		mergeto.add(new DropDownChoice<String>("mergeto", mergeToModel, branches));

+		form.add(mergeto.setVisible(!branches.isEmpty()));

 

 		form.add(new Button("update"));

 		Button cancel = new Button("cancel") {

diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
index 3f92eaa0..5e0e6820 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
@@ -115,7 +115,6 @@ public class TicketPage extends TicketBasePage {
 		super(params);

 

 		final UserModel user = GitBlitWebSession.get().getUser() == null ? UserModel.ANONYMOUS : GitBlitWebSession.get().getUser();

-		final boolean isAuthenticated = !UserModel.ANONYMOUS.equals(user) && user.isAuthenticated;

 		final RepositoryModel repository = getRepositoryModel();

 		final String id = WicketUtils.getObject(params);

 		long ticketId = Long.parseLong(id);

@@ -327,7 +326,7 @@ public class TicketPage extends TicketBasePage {
 		/*

 		 * UPDATE FORM (DISCUSSION TAB)

 		 */

-		if (isAuthenticated && app().tickets().isAcceptingTicketUpdates(repository)) {

+		if (user.canEdit(ticket, repository) && app().tickets().isAcceptingTicketUpdates(repository)) {

 			if (ticket.isOpen()) {

 				/*

 				 * OPEN TICKET