Skip to content

Fix/Story quotes not displaying on Explorer #57

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

m453h
Copy link

@m453h m453h commented May 8, 2025

This PR fixes an issue on the StoryQuoteTable where quotes for a selected story are never displayed. In order for this to work we need to deploy the Stanford CoreNLP API.
Screenshot 2025-05-08 at 08 53 54

Fixes #56

@m453h m453h self-assigned this May 8, 2025
@m453h m453h added the bug Something isn't working label May 8, 2025
@m453h m453h marked this pull request as ready for review May 13, 2025 12:34
@m453h m453h requested a review from thepsalmist May 13, 2025 12:42
@m453h m453h added this to COMMONS May 13, 2025
@github-project-automation github-project-automation bot moved this to 🚧 In Progress in COMMONS May 13, 2025
@m453h m453h moved this from 🚧 In Progress to 👀 In Review in COMMONS May 13, 2025
@m453h m453h requested a review from kilemensi May 13, 2025 12:43
@kilemensi
Copy link
Member

In order for this to work we need to deploy the Stanford CoreNLP API.

Shouldn't this PR come after we've deployed the CoreNLP API then @m453h ?

@m453h
Copy link
Author

m453h commented May 13, 2025

In order for this to work we need to deploy the Stanford CoreNLP API.

Shouldn't this PR come after we've deployed the CoreNLP API then @m453h ?

Hmm 🤔 not necessarily, if we deploy this as it is without the CoreNLP API, It wont behave any different since all requests for fetching quotes would still not get any response.

We could potentially have an issue if we deploy the CoreNLP API and set the CORENLP_URL in the explorer app before merging this one. I'd expect us to get some errors when we view story details because the response for extracting quotes might not include all the fields we are trying to access (In this PR, I used to the safer .get() method to access these fields to prevent these errors).

@kilemensi
Copy link
Member

What I mean @m453h is: Will users see any difference before vs after deploying this PR? If yes, then all good, I'm happy for us to proceed with the PR.

@m453h
Copy link
Author

m453h commented May 13, 2025

What I mean @m453h is: Will users see any difference before vs after deploying this PR? If yes, then all good, I'm happy for us to proceed with the PR.

They wouldn't see the difference until we deploy the Stanford CoreNLP API, I'll create a separate issue on backstage for the deployment of the API before we proceed with merging this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 👀 In Review
Development

Successfully merging this pull request may close these issues.

@CivicSignal - Story Quotes not displaying
2 participants