Skip to content

Fix related pubs bug#1017

Merged
allisonking merged 3 commits into
mainfrom
aking/1003/related-pubs-bug
Mar 4, 2025
Merged

Fix related pubs bug#1017
allisonking merged 3 commits into
mainfrom
aking/1003/related-pubs-bug

Conversation

@allisonking
Copy link
Copy Markdown
Contributor

Issue(s) Resolved

Closes #1003

High-level Explanation of PR

Our pub editor requires a list of pubs in a context so that it knows what pubs can be related to, and so it can render the titles of pubs that are already related. The problem is that when we query for pubs, we need to limit (in this case we limit to 30). So a pub might be related to another pub that is not in that initial 30, in which case the app can't render the pub's title since it wasn't returned with the query and breaks.

This fixes this initial problem by concatenating an existing pub's related pubs to the pubs sent to the pub context.

Test Plan

  1. Add a relation field (can be null)
  2. Add this field to a pubtype (i.e. Evaluation)
  3. Add this field to the pubtype's default form too
  4. Open an existing Evaluation pub (or create a new one)
  5. Create a related pub via the "Add Related Pub" button on the Evaluation pub
  6. Fill in any info to create the pub
  7. Now add one more pub of any type (this is just to put the created related pub out of range)
  8. Change the limit to 1
    https://github.com/pubpub/platform/blob/626e97e310537e74eeb8a77289514b7319d96d83/core/app/components/pubs/PubEditor/PubEditor.tsx#L166
  9. Open the pub in step 4
  10. Click "Update"
  11. The pub should still render properly

Screenshots (if applicable)

Notes

}
>
<div className="flex justify-center py-10">
<div className="prose max-w-prose flex-1">
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this originally came from #980 but it seems like just the flex-1 is enough to keep the width. adding prose changes a bunch of styles, including making <hr/>s have huge margins

image

prose is used for markdown, but I believe the various markdown places already have prose on them, i.e. StructuralElement
https://github.com/pubpub/platform/blob/0711707979fe1bc51874d120e33d6626b3f7be32/core/app/components/FormBuilder/FormElement.tsx#L167

Comment on lines +49 to +51
{element.schemaName !== CoreSchemaType.Null && (
<span>Please enter a value for this relationship.</span>
)}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just noticed it was still saying to fill out a field if the schema was null (and then it wouldn't render an input to fill out)


// For the Context, we want both the pubs from the initial pub query (which is limited)
// as well as the pubs related to this pub
const relatedPubs = pub ? pub.values.flatMap((v) => (v.relatedPub ? [v.relatedPub] : [])) : [];
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

TIL: you can use flatMap to get around how doing a .map(...).filter(value => !!value) doesn't let TS think you have Thing[] and TS will still think you have (Thing | null)[] even after the filter

https://stackoverflow.com/a/59726888

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

very hot tip, the .filter typing is so annoying!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i think they did fix this in a recent release, if you do .filter(value=>value != null) specifically!

Copy link
Copy Markdown
Member

@allisonking allisonking requested review from 3mcd and kalilsn March 4, 2025 18:01
@allisonking allisonking marked this pull request as ready for review March 4, 2025 18:45
Copy link
Copy Markdown
Contributor

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

Works great! Tested and confirmed the bug on main (with limit 1) and that it works on your branch

@allisonking allisonking merged commit 285a3f3 into main Mar 4, 2025
@allisonking allisonking deleted the aking/1003/related-pubs-bug branch March 4, 2025 20:23
allisonking added a commit that referenced this pull request Mar 6, 2025
* Remove prose class which added unwanted styling

* Concat related pubs to pub context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Editing a pub internally breaks if related pub doesn't happen to be within the limit

3 participants