Skip to content
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

HOTFIX: display synchronize new remote project url errors #1433

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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

## [22.09.5] - Unreleased
* [Developer/UI]: Fixed bug preventing a manager of a user group from adding new members when this manager is a collaborator on one of these users projects. Also, fixed issue with a user group member added with an owner role for a group was set with a member role. See [PR 1431](https://github.com/phac-nml/irida/pull/1431)
* [Developer/UI]: Updated synchronize new remote project page to display http errors when setting the url manually and an error is encountered. See [PR 1433](https://github.com/phac-nml/irida/pull/1433)


## [22.09.4] - 2022/11/14
* [REST]: Fixed issue with project/samples api response missing samples when a sample has a default sequencing object. See [PR 1413](https://github.com/phac-nml/irida/pull/1413)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package ca.corefacility.bioinformatics.irida.exceptions;

import org.springframework.http.HttpStatus;

import ca.corefacility.bioinformatics.irida.model.RemoteAPI;
import ca.corefacility.bioinformatics.irida.model.RemoteAPIToken;

Expand All @@ -14,6 +16,7 @@ public class IridaOAuthException extends RuntimeException {

private static final long serialVersionUID = 5281201199554307578L;
private RemoteAPI remoteAPI;
private HttpStatus httpStatusCode;

/**
* Create a new IridaOAuthException with the given message and service
Expand All @@ -29,6 +32,13 @@ public IridaOAuthException(String message, RemoteAPI remoteAPI) {
this.remoteAPI = remoteAPI;
}

public IridaOAuthException(String message, HttpStatus httpStatusCode, RemoteAPI remoteAPI) {
super(message);
this.httpStatusCode = httpStatusCode;
this.remoteAPI = remoteAPI;

}

/**
* Create a new IridaOAuthException with the given message and service
*
Expand Down Expand Up @@ -63,4 +73,12 @@ public RemoteAPI getRemoteAPI() {
public void setRemoteAPI(RemoteAPI remoteAPI) {
this.remoteAPI = remoteAPI;
}

public HttpStatus getHttpStatusCode() {
return httpStatusCode;
}

public void setHttpStatusCode(HttpStatus httpStatusCode) {
this.httpStatusCode = httpStatusCode;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ public void handleError(ClientHttpResponse response) throws IOException {
logger.trace("Checking error type " + statusCode.toString());
switch (statusCode) {
case UNAUTHORIZED:
logger.trace("Throwing new IridaOAuthException for this error");
throw new IridaOAuthException("User is unauthorized for this service", remoteAPI);
logger.trace("Throwing new IridaOAuthException (Unauthorized) for this error");
throw new IridaOAuthException("User is unauthorized for this service", statusCode, remoteAPI);
case FORBIDDEN:
logger.trace("Throwing new IridaOAuthException (Forbidden) for this error");
throw new IridaOAuthException("User is forbidden from accessing this resource", statusCode, remoteAPI);
case NOT_FOUND:
logger.trace("Throwing new IridaOAuthException for this error");
throw new IridaOAuthException("User is unauthorized for this service", remoteAPI);
throw new IridaOAuthException("Resource not found", statusCode, remoteAPI);
default:
logger.trace("Passing error to superclass");
super.handleError(response);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,11 @@ public ResponseEntity<List<RemoteProjectModel>> getProjectsForAPI(@PathVariable
* @return status of created the new remote project
*/
@PostMapping("/project")
public ResponseEntity<AjaxResponse> createSynchronizedProject(@RequestBody CreateRemoteProjectRequest request) {
public ResponseEntity<AjaxResponse> createSynchronizedProject(@RequestBody CreateRemoteProjectRequest request, Locale locale) {
try {
return ResponseEntity.ok(service.createSynchronizedProject(request));
return ResponseEntity.ok(new AjaxCreateItemSuccessResponse(service.createSynchronizedProject(request, locale)));
} catch (IridaOAuthException e) {
return ResponseEntity.status(HttpStatus.FORBIDDEN).body(new AjaxErrorResponse(e.getMessage()));
} catch (EntityNotFoundException e) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(new AjaxErrorResponse(e.getMessage()));
return ResponseEntity.status(e.getHttpStatusCode()).body(new AjaxErrorResponse(e.getMessage()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.MessageSource;
import org.springframework.stereotype.Component;

import ca.corefacility.bioinformatics.irida.exceptions.IridaOAuthException;
Expand All @@ -14,10 +16,10 @@
import ca.corefacility.bioinformatics.irida.model.project.Project;
import ca.corefacility.bioinformatics.irida.model.project.ProjectSyncFrequency;
import ca.corefacility.bioinformatics.irida.model.remote.RemoteStatus;
import ca.corefacility.bioinformatics.irida.ria.web.ajax.dto.ajax.AjaxCreateItemSuccessResponse;
import ca.corefacility.bioinformatics.irida.ria.web.ajax.dto.remote.CreateRemoteProjectRequest;
import ca.corefacility.bioinformatics.irida.ria.web.ajax.dto.remote.RemoteAPIModel;
import ca.corefacility.bioinformatics.irida.ria.web.ajax.dto.remote.RemoteProjectModel;

import ca.corefacility.bioinformatics.irida.service.ProjectService;
import ca.corefacility.bioinformatics.irida.service.RemoteAPIService;
import ca.corefacility.bioinformatics.irida.service.RemoteAPITokenService;
Expand All @@ -33,13 +35,16 @@ public class UIRemoteAPIService {
private final ProjectRemoteService projectRemoteService;
private final ProjectService projectService;

private MessageSource messageSource;

@Autowired
public UIRemoteAPIService(RemoteAPIService remoteAPIService, RemoteAPITokenService tokenService,
ProjectRemoteService projectRemoteService, ProjectService projectService) {
ProjectRemoteService projectRemoteService, ProjectService projectService, MessageSource messageSource) {
this.remoteAPIService = remoteAPIService;
this.tokenService = tokenService;
this.projectRemoteService = projectRemoteService;
this.projectService = projectService;
this.messageSource = messageSource;
}

/**
Expand Down Expand Up @@ -114,19 +119,34 @@ public List<RemoteProjectModel> getProjectsForAPI(Long remoteId) {
* @param request Details about the remote project to synchronize
* @return the result of adding the new remote project
*/
public AjaxCreateItemSuccessResponse createSynchronizedProject(CreateRemoteProjectRequest request) {
Project project = projectRemoteService.read(request.getUrl());
project.setId(null);
if (request.getFrequency()
.equals(ProjectSyncFrequency.NEVER)) {
project.getRemoteStatus()
.setSyncStatus(RemoteStatus.SyncStatus.UNSYNCHRONIZED);
} else {
project.getRemoteStatus()
.setSyncStatus(RemoteStatus.SyncStatus.MARKED);
public Long createSynchronizedProject(CreateRemoteProjectRequest request, Locale locale) {
try {
Project project = projectRemoteService.read(request.getUrl());
project.setId(null);
if (request.getFrequency().equals(ProjectSyncFrequency.NEVER)) {
project.getRemoteStatus().setSyncStatus(RemoteStatus.SyncStatus.UNSYNCHRONIZED);
} else {
project.getRemoteStatus().setSyncStatus(RemoteStatus.SyncStatus.MARKED);
}
project.setSyncFrequency(request.getFrequency());
project = projectService.create(project);
return project.getId();
} catch(IridaOAuthException e) {
String errorMessage;
switch (e.getHttpStatusCode()) {
case UNAUTHORIZED:
errorMessage = messageSource.getMessage("IridaOAuthErrorHandler.unauthorized", new Object[] {}, locale);
throw new IridaOAuthException(errorMessage, e.getHttpStatusCode(), remoteAPIService.getRemoteAPIForUrl(request.getUrl()));
case FORBIDDEN:
errorMessage = messageSource.getMessage("IridaOAuthErrorHandler.forbidden", new Object[] {}, locale);
throw new IridaOAuthException(errorMessage, e.getHttpStatusCode(), remoteAPIService.getRemoteAPIForUrl(request.getUrl()));
case NOT_FOUND:
errorMessage = messageSource.getMessage("IridaOAuthErrorHandler.notFound", new Object[] {}, locale);
throw new IridaOAuthException(errorMessage, e.getHttpStatusCode(), remoteAPIService.getRemoteAPIForUrl(request.getUrl()));
default:
errorMessage = e.getMessage();
throw new IridaOAuthException(errorMessage, e.getHttpStatusCode(), remoteAPIService.getRemoteAPIForUrl(request.getUrl()));
}
}
project.setSyncFrequency(request.getFrequency());
project = projectService.create(project);
return new AjaxCreateItemSuccessResponse(project.getId());
}
}
4 changes: 4 additions & 0 deletions src/main/resources/i18n/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -3023,3 +3023,7 @@ AnalysisState.ERROR=Error
# InputWithOptions #
#=========================================================================================#
InputWithOptions.required={0} is required to run this pipeline. Please select one of the options.

IridaOAuthErrorHandler.unauthorized=User is unauthorized for this service
IridaOAuthErrorHandler.forbidden=Access denied. User is forbidden from accessing this resource
IridaOAuthErrorHandler.notFound=Resource not found. A resource with the given identifier could not be found
5 changes: 4 additions & 1 deletion src/main/webapp/resources/js/apis/remote-api/remote-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,8 @@ export function getProjectsForAPI({ id }) {
export function createSynchronizedProject({ url, frequency }) {
return axios
.post(`${BASE_URL}/project`, { url, frequency })
.then(({ data }) => data);
.then(({ data }) => data)
.catch((error) => {
return Promise.reject(error.response.data.error);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { SyncFrequencySelect } from "./SyncFrequencySelect";
export function CreateRemoteProjectSyncForm() {
const [apis, setApis] = useState([]);
const [selectedApi, setSelectedApi] = useState();
const [newRemoteProjectUrlError, setNewRemoteProjectUrlError] =
useState(null);
const [projects, setProjects] = useState([]);
const [connected, setConnected] = useState();
const [manual, setManual] = useState(false);
Expand Down Expand Up @@ -53,12 +55,15 @@ export function CreateRemoteProjectSyncForm() {

const createRemote = () => {
form.validateFields().then(({ frequency, url }) => {
setNewRemoteProjectUrlError(null);
createSynchronizedProject({
url,
frequency,
}).then(
({ id }) => (window.location.href = setBaseUrl(`/projects/${id}`))
);
})
.then(
({ id }) => (window.location.href = setBaseUrl(`/projects/${id}`))
)
.catch((error) => setNewRemoteProjectUrlError(error));
});
};

Expand Down Expand Up @@ -106,6 +111,10 @@ export function CreateRemoteProjectSyncForm() {
/>
</Form.Item>
<Form.Item
validateStatus={newRemoteProjectUrlError !== null && "error"}
help={
newRemoteProjectUrlError !== null && newRemoteProjectUrlError
}
rules={[
{
required: true,
Expand All @@ -115,7 +124,13 @@ export function CreateRemoteProjectSyncForm() {
label={
<span>
{i18n("NewProjectSync.remoteUrl")}
<Checkbox onChange={(e) => setManual(e.target.checked)}>
<Checkbox
className="t-remote-project-url-checkbox"
onChange={(e) => {
setManual(e.target.checked);
setNewRemoteProjectUrlError(null);
}}
>
{i18n("NewProjectSync.remoteUrl.manual")}
<HelpPopover
content={<div>{i18n("NewProjectSync.url.help")}</div>}
Expand All @@ -125,7 +140,11 @@ export function CreateRemoteProjectSyncForm() {
}
name="url"
>
<Input className="t-project-url" disabled={!manual} />
<Input
className="t-project-url"
disabled={!manual}
onChange={() => setNewRemoteProjectUrlError(null)}
/>
</Form.Item>
<SyncFrequencySelect />
</>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ca.corefacility.bioinformatics.irida.ria.integration.pages.projects;

import java.time.Duration;
import java.util.List;

import org.openqa.selenium.By;
import org.openqa.selenium.Keys;
Expand Down Expand Up @@ -78,4 +79,37 @@ public void submitProject() {
waitForTime(500);
}

public boolean isResourceNotFoundErrorMessageDisplayed() {
List<WebElement> elements = driver.findElements(By.className("ant-form-item-explain-error"));

if(elements.size() == 1) {
return elements.get(0).getText().contains("Resource not found");
}
return false;
}

public boolean isAccessDeniedErrorMessageDisplayed() {
List<WebElement> elements = driver.findElements(By.className("ant-form-item-explain-error"));

if(elements.size() == 1) {
return elements.get(0).getText().contains("Access denied");
}
return false;
}

public void clickSetUrlManuallyCheckbox() {
WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
WebElement element = driver.findElement(By.className("t-remote-project-url-checkbox"));
wait.until(ExpectedConditions.elementToBeClickable(element));
element.click();
waitForTime(500);
}

public void setRemoteProjectUrl(String projectUrl) {
WebElement element = driver.findElement(By.className("t-project-url"));
element.sendKeys(Keys.CONTROL + "a", Keys.DELETE);
element.sendKeys(projectUrl);
waitForTime(500);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
public class RemoteAPIsPage extends AbstractPage {
private static final String RELATIVE_URL = "remote_api";

@FindBy(css = ".t-remoteapi-table table")
@FindBy(className = "t-remoteapi-table")
private WebElement table;

@FindBy(className = "t-add-remote-api-btn")
Expand Down Expand Up @@ -120,7 +120,10 @@ public void connectToRemoteAPI(String clientName) {
for (WebElement row : rows) {
WebElement nameCell = row.findElement(By.className("t-api-name"));
if (nameCell != null && nameCell.getText().equals(clientName)) {
row.findElement(By.className("t-remote-status-connect")).click();
WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(2));
WebElement connectBtn = row.findElement(By.className("t-remote-status-connect"));
wait.until(ExpectedConditions.elementToBeClickable(connectBtn));
connectBtn.click();
waitForTime(400);
clickAuthorize();
}
Expand Down
Loading