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

Don't create unordered news and events folder by default #553

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rodfersou
Copy link
Member

closes #552

@rodfersou rodfersou self-assigned this May 4, 2020
@mister-roboto
Copy link

@rodfersou thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@rodfersou
Copy link
Member Author

@jenkins-plone-org please run jobs

@rodfersou rodfersou marked this pull request as ready for review May 4, 2020 09:05
@rodfersou rodfersou requested review from tisto and sneridagh May 4, 2020 09:09
@jensens
Copy link
Member

jensens commented May 4, 2020

May you give us more details on this change? Even when following the two issues I do not get the nature of the problem this may solve. Also:

  • What does unordered mean in context of IOrdering?
  • Why was it there and
  • what is set by default now, after removing the lines?
  • What is changing for the users?

@rodfersou
Copy link
Member Author

rodfersou commented May 4, 2020

  • What does unordered mean in context of IOrdering?

with Unordered folder it is impossible to get the object order in a folder

  • Why was it there and

good question.. don't know, but I'm assuming it is a mistake

  • what is set by default now, after removing the lines?

if remove those lines will use the default ordering

  • What is changing for the users?

no change for users, but for restapi it is possible to get the next and previous item at those folders.

@rodfersou rodfersou requested a review from jensens May 4, 2020 13:03
@jensens jensens requested a review from pbauer May 6, 2020 11:19
Copy link
Member

@jensens jensens left a comment

Choose a reason for hiding this comment

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

Given tests go green, I feel fine.

@rodfersou
Copy link
Member Author

@jenkins-plone-org please run jobs

@rodfersou
Copy link
Member Author

well.. there are some tests preventing this change at Products.CMFPlone

will take a look at it if can find the reason to be unordered, or if can remove those tests

@rodfersou
Copy link
Member Author

@jensens I'm looking the tests, this comment make me feel that should not touch members folders, but the others are okay to become unordered.

@rodfersou
Copy link
Member Author

looks like Jenkins is not working now.. will try again other day 😄

@mauritsvanrees
Copy link
Member

Can you edit the news snippet to explain that this is only about the standard news and events folders?
Currently it sounds like this is a change for all folders, so also folders that an editor creates manually, which would mean a major, potentially breaking change.

I guess I can understand why news and events were unordered: by default it makes no sense to manually order items here, because you simply view them through the Collection, which orders them with most recent news item first, or nearest future event first.
Should be okay to change this.

@rodfersou rodfersou changed the title Don't create unordered folder by default Don't create unordered news and events folder by default Jul 21, 2020
@rodfersou
Copy link
Member Author

related to plone/Products.CMFPlone#3142

@jensens
Copy link
Member

jensens commented Jul 30, 2020

@jenkins-plone-org please run jobs

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.

Don't create unordered folders by default
4 participants