-
Notifications
You must be signed in to change notification settings - Fork 9
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
Prevent errors when processing additional rdf metadata #2496
base: dev
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for regen-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This solves the issue of the page crashing. Compare these credit batch pages that have json-ld metadata with multiple https://dev.app.regen.network/credit-batches/MBS01-001-20200101-20210101-001 to |
This doesn't crash anymore, but the metadata is not displayed in "additional info" since it now excludes the whole graph, we would probably need to update our |
Yes, but could we get this merged so the page stops crashing? It would
still be useful to show these pages to clients while we are testing
…On Tue, Oct 8, 2024, 12:02 AM Marie Gauthier ***@***.***> wrote:
This solves the issue of the page crashing. Compare these credit batch
pages that have json-ld metadata with multiple @graph components (valid
json-ld!) returned via our data resolver:
https://dev.app.regen.network/credit-batches/MBS01-001-20200101-20210101-001
to
https://deploy-preview-2496--regen-marketplace.netlify.app/credit-batches/MBS01-001-20200101-20210101-001
This doesn't crash anymore, but the metadata is not displayed in
"additional info" since it now excludes the whole graph, we would probably
need to update our jsonLdCompact to work with @graphs
—
Reply to this email directly, view it on GitHub
<#2496 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXY7QY7ZT465VJTRDDD4XTZ2N7Q7AVCNFSM6AAAAABPQ5YIFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJZGAYTANBQHE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but could we get this merged so the page stops crashing? It would still be useful to show these pages to clients while we are testing
Sure, pre-approving. Could you just put a comment in the code about that?
Sure, I'm away from my computer but can do that next week
…On Tue, Oct 8, 2024, 10:36 PM Marie Gauthier ***@***.***> wrote:
***@***.**** approved this pull request.
Yes, but could we get this merged so the page stops crashing? It would
still be useful to show these pages to clients while we are testing
Sure, pre-approving. Could you just put a comment in the code about that?
—
Reply to this email directly, view it on GitHub
<#2496 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAXY7Q4I3N45BNZ5RCJRINLZ2S6EZAVCNFSM6AAAAABPQ5YIFOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGNJWGA3DEOBTHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
It's possible that the jsonld context fully defines a field label so that it doesn't contain a "regen:" prefix
// Remove the field prefix and add space before capital letters. | ||
return fieldName.replace(/[^:]*\:/, '').replace(/([A-Z])/g, ' $1'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes another bug we discovered where the page crashes if the field key doesn't have a colon prefix. This is a simple change to safely remove the first prefix:
if one exists.
Compare: https://dev.app.regen.network/credit-batches/MBS01-001-20240601-20340531-002
Description
Testing to see if this fixes #2495
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
How to test
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...