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

Make iThenticate reports work with new TCA integration #172

Merged
merged 1 commit into from
Aug 10, 2023

Conversation

MusikAnimal
Copy link
Member

No description provided.

src/Controller/AppController.php Outdated Show resolved Hide resolved
src/Controller/AppController.php Show resolved Hide resolved
[
'json' => [
'viewer_user_id' => ':system:',
'locale' => $intuition->getLang(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure it is one of the supported locales.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs said it would fallback to en if unsupported, so I figure this couldn't hurt

'POST',
"https://{$config['domain']}/api/v1/submissions/$id/viewer-url",
[
'json' => [
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider whether any viewer_permissions should be set. Some could be useful to reviewers.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added may_view_submission_full_source, may_view_match_submission_info, may_view_document_details_panel and may_view_sections_exclusion_panel. The others didn't seem to do anything or didn't work.

* @param string $id
* @return RedirectResponse
*/
private function redirectToTcaViewer(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TCA can be down (e.g., maintenance). Does this need any special handling for that?

Copy link
Member Author

@MusikAnimal MusikAnimal Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added automatic retries up to 5 times. I'm only checking if the response from Turnitin is not 200, so it's possible we'll needlessly retry when it's something on our end (i.e. bad credentials), but regardless we'll get an email (once I set that up) so we'll be able to diagnose and fix accordingly.

@MusikAnimal MusikAnimal force-pushed the tca-viewer branch 3 times, most recently from b30a048 to acaddd8 Compare August 9, 2023 19:27
@JJMC89
Copy link

JJMC89 commented Aug 10, 2023

I temporarily deployed 3cd248c and got 500 when trying to view reports (ithenticate route). Rolled back to 6b18613.

@MusikAnimal
Copy link
Member Author

I temporarily deployed 3cd248c and got 500 when trying to view reports (ithenticate route). Rolled back to 6b18613.

I've re-deloyed it and it seems to be working fine.

I checked the logs (~/var/log/prod.log) and the errors I see are a bit baffling. One thing I did that probably fixed it was ran webservice shell then composer install, which in turn will clear the cache (you can do so manually in the container with ./bin/console c:c --env=prod). There were composer changes but it was just adding an ext-json requirement which was already met, so everything should have worked with a simple checkout of the commit.

@MusikAnimal MusikAnimal merged commit b4e7c8b into master Aug 10, 2023
10 checks passed
@MusikAnimal MusikAnimal deleted the tca-viewer branch August 10, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants