Conversation
… frontend - need to fix api errors
…ix collections and bibliographies
…rt from citesphere working
…- need to remove debug statements, clean code
|
Can one of the admins verify this patch? |
1 similar comment
|
Can one of the admins verify this patch? |
|
Resolve conflicts please |
jdamerow
left a comment
jdamerow
left a comment
There was a problem hiding this comment.
gotta run, but I'll leave it with this for now.
| private String accessToken; | ||
| private String refreshToken; | ||
| private String username; | ||
| private String password; |
There was a problem hiding this comment.
there should be no password needed here.
There was a problem hiding this comment.
Those entries should probably not be removed
|
|
||
| if (!"oauth".equals(authTokenObject.getAuthType()) && | ||
| !"basic".equals(authTokenObject.getAuthType())) { | ||
| throw new IllegalArgumentException("authType should be either oauth or basic"); |
There was a problem hiding this comment.
there should be no basic authentication with citesphere.
|
|
||
| try (Response response = client.newCall(request).execute()) { | ||
| // Check for unauthorized or forbidden responses | ||
| if (response.code() == 401 || response.code() == 403) { |
There was a problem hiding this comment.
the http codes are defined in constants (not sure what the class is called, it depends on the included library, maybe maybe HttpStatus or something along those lines?)
| } | ||
| } catch (CitesphereTokenException e) { | ||
| throw e; // Re-throw token exceptions | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
what is throwing Exception (specific exceptions should be caught)? If it's not thrown anywhere, then this should be removed/replace with specific exceptions and the catch block above catching CitesphereTokenException would also not be necessary.
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> checkTest() { |
| } | ||
|
|
||
| @Override | ||
| public Map<String, Object> checkAccess(String documentId) { |
There was a problem hiding this comment.
I don't think this is needed, is it?
| errorMap.put("token_expired", e.isTokenExpired()); | ||
| return errorMap; | ||
| } | ||
| } |
There was a problem hiding this comment.
many of these methods look almost identical (except the line that builds the url), so there should be one method with this call and the other methods would just build the url and call that method.
| .build(); | ||
|
|
||
| try (Response response = client.newCall(request).execute()) { | ||
| if (response.code() == 401 || response.code() == 403) { |
| } | ||
| } catch (CitesphereTokenException e) { | ||
| throw e; | ||
| } catch (Exception e) { |
…ation, replaced HTTP status numbers with status constants, removed file upload code, and reverted gitignore to orignal
…tus constants, removed check Access
4 participants
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
It should be possible to pull in bibliography entries from citesphere
Description
Instead of entering bibliography entries by hand, the user would select entries from Citesphere and those would then be imported. Since Citesphere doesn't have a search endpoint yet, the user would when adding a bibliography block, browse their groups in citesphere and select entries to import. So:
For now, this would be a one time import. We'll worry about updating references from Citesphere later.
The code that pull data from Citesphere should be its own plugin (have Julia create the basic plugin).
VSPC-189
Anything else the reviewer needs to know?
... describe here ...