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.

Sign up for GitHub

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

Jump to bottom

(Synchronize redundant block id in listing block on pasting) Fix duplicating listing block #4239

Merged
merged 10 commits into from Apr 19, 2023
Merged

(Synchronize redundant block id in listing block on pasting) Fix duplicating listing block #4239

merged 10 commits into from Apr 19, 2023

Conversation

ksuess
Copy link
Member

@ksuess ksuess commented Jan 9, 2023

…ockid].block
fix #4234

@netlify
Copy link

netlify bot commented Jan 9, 2023 •
edited

✅ Deploy Preview for volto ready!

Name Link
🔨 Latest commit 30c4b51
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/643fc68f4e98410008eec0bd
😎 Deploy Preview https://deploy-preview-4239--volto.netlify.app/blocks/ssr.html
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cypress
Copy link

cypress bot commented Jan 9, 2023 •
edited

Passing run #4985 ↗︎

0 493 20 0 Flakiness 0

Details:

Merge branch 'master' into multiple-listing-blocks
Project: Volto Commit: 30c4b5166f
Status: Passed Duration: 14:26 💡
Started: Apr 19, 2023 10:50 AM Ended: Apr 19, 2023 11:04 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@ksuess
Copy link
Member Author

ksuess commented Jan 9, 2023

Maybe this bug fix is a breaking one: If somebody relies in getAsyncData on block uid in data, they have to rewrite their function. See https://github.com/plone/volto/blob/05f09ce261f78dfddd1bb4bcd3ed1c965b004337/docs/source/blocks/ssr.md

@ksuess ksuess changed the title Synchronize redundant block id in listing block on pasting: blocks[bl… (Synchronize redundant block id in listing block on pasting) Fix duplicating listing block Jan 9, 2023
tiberiuichim
tiberiuichim approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@tiberiuichim tiberiuichim left a comment

Choose a reason for hiding this comment

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

The signature of getQueryStringResults hasn't changed. The documentation change is only a "fantasy" example, it would not represent a breaking change, IMHO.

image

@sneridagh
Copy link
Member

@tiberiuichim @ksuess we are not persisting block anymore in the block data... I'd say it's breaking :/ I know it shouldn't be there, but still... maybe someone is relying on it (eg. shadows).

Let's discuss it in the next meeting.

@ksuess ksuess self-assigned this Jan 13, 2023
@sneridagh
Copy link
Member

Volto Team Meeting:
We will try to make the PR not breaking. We could also have a cleaning (breaking) PR for 17.

@sneridagh
Copy link
Member

@ksuess I'd merge this right away, as a breaking change. Then afterwards work it out if it make sense for 16 too. Are you ok?

@sneridagh
Copy link
Member

@ksuess Would you change the type of change? and possibly write something brief in the upgrade guide? Thanks!

@ksuess
Copy link
Member Author

ksuess commented Mar 16, 2023

I thought about it back and forth. I do not see anymore a breaking change.

What is fixed here, is that the listing block does not save the block id in blocks data anymore.
If somebody still does this, no problem occures, as this fix is just taking the id from blocks props, which is always available.
Do you agree?

@ksuess ksuess merged commit 0e62195 into master Apr 19, 2023
55 checks passed
@ksuess ksuess deleted the multiple-listing-blocks branch April 19, 2023 11:06
ionlizarazu pushed a commit that referenced this pull request Apr 20, 2023
@ksuess @ionlizarazu
(Synchronize redundant block id in listing block on pasting) Fix dupl…
f8de286 
…icating listing block (#4239)

Obvoiusly no objections anymore.
@davisagli davisagli mentioned this pull request May 5, 2023
Open
sneridagh added a commit that referenced this pull request May 10, 2023
@sneridagh
Merge branch 'master' into poc-blocks-navigation-tab
5e1a947 
* master: (83 commits)
  Apply suggestion from browser for password field (#4524)
  (fix):Object.normaliseMail: Cannot read properties of null (#4558)
  Fix link in Volto, remove from linkcheck ignore in Documentation v6.0 (#4742)
  added documentation regarding the static middleware #4518 (#4736)
  Closes issue #4567 (#4570)
  Fix whitespace in locales created by the generator (#4737)
  Tidy up from synch with 16.x.x (#4728)
  Release notes from 16.20.2, 16.20.3, 16.20.4 (#4729)
  Use new URL 6.docs.plone.org (#4726)
  Security upgrade for momentjs (#4715)
  Don't decode querystring while adding apiExpanders (#4719)
  (Synchronize redundant block id in listing block on pasting) Fix duplicating listing block (#4239)
  Translate add-on control panel (cleaned up PR) (#4620)
  Fix Move to top of folder ordering in folder content view by searchin… (#4709)
  Fix robot.txt - the sitemap link should respect x-forwarded headers (#4704)
  Refactor faulty reorder elements in ObjectBrowserList widget (#4703)
  Release changelog notes for 16.20.1
  Release 17.0.0-alpha.5
  Generate a split sitemap (also fix robots.txt) (#4639)
  Fix search block in edit mode re-queries multiple blocks with an empty search text (#4694)
  ...
sneridagh added a commit that referenced this pull request May 11, 2023
@sneridagh
Merge branch 'master' into sort-controlpanel-by-title
a20dd5a 
* master: (185 commits)
  fix: unresponsive add page (#4507)
  Apply suggestion from browser for password field (#4524)
  (fix):Object.normaliseMail: Cannot read properties of null (#4558)
  Fix link in Volto, remove from linkcheck ignore in Documentation v6.0 (#4742)
  added documentation regarding the static middleware #4518 (#4736)
  Closes issue #4567 (#4570)
  Fix whitespace in locales created by the generator (#4737)
  Tidy up from synch with 16.x.x (#4728)
  Release notes from 16.20.2, 16.20.3, 16.20.4 (#4729)
  Use new URL 6.docs.plone.org (#4726)
  Security upgrade for momentjs (#4715)
  Don't decode querystring while adding apiExpanders (#4719)
  (Synchronize redundant block id in listing block on pasting) Fix duplicating listing block (#4239)
  Translate add-on control panel (cleaned up PR) (#4620)
  Fix Move to top of folder ordering in folder content view by searchin… (#4709)
  Fix robot.txt - the sitemap link should respect x-forwarded headers (#4704)
  Refactor faulty reorder elements in ObjectBrowserList widget (#4703)
  Release changelog notes for 16.20.1
  Release 17.0.0-alpha.5
  Generate a split sitemap (also fix robots.txt) (#4639)
  ...
@davisagli davisagli mentioned this pull request Jul 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davisagli davisagli davisagli left review comments

@tiberiuichim tiberiuichim tiberiuichim approved these changes

@sneridagh sneridagh Awaiting requested review from sneridagh

Assignees

@ksuess ksuess

Labels
None yet
Projects
Volto Team Meeting
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multiple listing blocks: Rendering fails for copy-pasted listing blocks
4 participants
@ksuess @sneridagh @davisagli @tiberiuichim