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

Fix falining AT Collection creation when using api.content.create. #414

Merged
merged 6 commits into from
Sep 14, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ Bug fixes:
- fix typos in doc strings
[tkimnguyen]

- Fix falining AT Collection creation when using api.content.create.
Copy link
Member

Choose a reason for hiding this comment

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

typo

[gbastien]


1.8.4 (2018-04-24)
------------------
Expand Down
4 changes: 2 additions & 2 deletions src/plone/api/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@ def create(
# rename-after-creation and such
# Passing values as a dict with None values so values set by
# invokeFactory don't get overridden.
# None: None is required so that bool(values) is True.
content.processForm(values={None: None})
# '': '' is required so that bool(values) is True.
content.processForm(values={'': ''})

if not id or (safe_id and id):
# Create a new id from title
Expand Down
58 changes: 58 additions & 0 deletions src/plone/api/tests/test_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,64 @@ def test_create_at_with_title_in_request(self):

self.assertEqual(page.title, 'Test document')

@unittest.skipIf(HAS_PACONTENTYPES, 'Archetypes only')

Choose a reason for hiding this comment

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

@gbastien thinking minor, I think this test should run with dx too. Collection creation should work with dx. You can remove this condition and change the name of the test.

def test_create_at_collection(self):
"""Test create at Collecition."""
Copy link
Member

Choose a reason for hiding this comment

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

typo

collection = api.content.create(
container=self.portal,
type='Collection',
title='Mandelbrot set',
description='Image gallery of a zoom sequence',
query=[
{
'i': 'Type',
'o': 'plone.app.querystring.operation.string.is',
'v': ['Image'],
},
],
)
self.assertEqual(collection.Title(), 'Mandelbrot set')

@unittest.skipIf(HAS_PACONTENTYPES, 'Archetypes only')
def test_create_at_event(self):
"""https://github.com/plone/plone.api/issues/364"""
from DateTime import DateTime
today = DateTime()
tomorrow = today + 1
event = api.content.create(
container=self.portal,
type='Event',
title=u'My event',
startDate=today,
endDate=tomorrow,
)
self.assertEqual(event.startDate, today)
self.assertEqual(event.endDate, tomorrow)
results = api.content.find(Title=u'My event')
self.assertEqual(len(results), 1)
self.assertEqual(results[0].start, today)
self.assertEqual(results[0].end, tomorrow)

@unittest.skipUnless(HAS_PACONTENTYPES, 'Dexterity only')
def test_create_dx_event(self):
"""https://github.com/plone/plone.app.contenttypes/issues/465"""

Choose a reason for hiding this comment

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

@hvelarde what's the relationship between plone/plone.app.contenttypes#465 and plone.api? It seems to me more a problem of plone.app.contenttypes.

I think this test can continue but without reference to plone.app.contenttypes.

Copy link
Member

@hvelarde hvelarde Sep 6, 2018

Choose a reason for hiding this comment

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

IIRC, I was unable to reproduce it on a clean Plone installation; so maybe it's not related at all; we may remove the reference.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fixed the typos and run test for AT and DX and Travis is green.

I kept the test regarding the event creation, do not know if it is necessary now that we have a test with Collection creation for AT and DX?

Choose a reason for hiding this comment

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

@gbastien this event testing is necessary to avoid regression #364. I think it is necessary now only to remove the reference:

plone/plone.app.contenttypes#465

Choose a reason for hiding this comment

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

In fact what avoids #364 is the AT test. But it's good to have an event creation test with DX

Copy link
Member Author

Choose a reason for hiding this comment

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

OK so I would remove the test_create_at_event, unnecessary now because of test_create_collection?

Choose a reason for hiding this comment

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

@gbastien no. test_create_at_event avoid regression #364. test_create_collection not avoid that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, so let's merge? :-)

Choose a reason for hiding this comment

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

@gbastien I think it is necessary now only to remove the reference:

plone/plone.app.contenttypes#465

in docstring.

import datetime
today = datetime.datetime.now()
tomorrow = today + datetime.timedelta(days=1)
event = api.content.create(
container=self.portal,
type='Event',
title=u'My event',
start=today,
end=tomorrow,
)
self.assertEqual(event.start, today)
self.assertEqual(event.end, tomorrow)
results = api.content.find(Title=u'My event')
self.assertEqual(len(results), 1)
self.assertEqual(results[0].start, today)
self.assertEqual(results[0].end, tomorrow)

def test_get_constraints(self):
"""Test the constraints when content is fetched with get."""

Expand Down