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

Fixed changing the order in a list tile by drag and drop. #881

Merged
merged 1 commit into from Sep 16, 2020

Conversation

mauritsvanrees
Copy link
Member

Changing the order in a list tile on the Compose tab would seemingly work at first, but nothing was saved.
The wrong element was taken as base (the whole html document instead of the tile).
And iterating over the children to get their uuids was done wrong, trying to take a uuid attribute from always that same wrong element.

Tested in Plone 4.3 and 5.1.

@mauritsvanrees
Copy link
Member Author

This fails on Travis only on 5.1, with this error:

While:
1904  Installing.
1905  Getting section code-analysis.
1906  Initializing section code-analysis.
1907  Installing recipe plone.recipe.codeanalysis[recommended].
1908
1909An internal error occurred due to a bug in either zc.buildout or in a
1910recipe being used:
1911Traceback (most recent call last):
1912  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 2174, in main
1913    getattr(buildout, command)(args)
1914  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 701, in install
1915    [self[part]['recipe'] for part in install_parts]
1916  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 1324, in __getitem__
1917    options._initialize()
1918  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 1432, in _initialize
1919    self.initialize()
1920  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 1438, in initialize
1921    recipe_class = _install_and_load(reqs, 'zc.buildout', entry, buildout)
1922  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/buildout.py", line 1388, in _install_and_load
1923    allow_hosts=buildout._allow_hosts
1924  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/easy_install.py", line 957, in install
1925    return installer.install(specs, working_set)
1926  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/easy_install.py", line 703, in install
1927    req = self._constrain(current_requirement)
1928  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/easy_install.py", line 657, in _constrain
1929    requirement)
1930  File "/home/travis/build/collective/collective.cover/eggs/zc.buildout-2.13.3-py2.7.egg/zc/buildout/easy_install.py", line 1614, in _constrained_requirement
1931    str(requirement) + ',' + constraint
1932  File "/home/travis/virtualenv/python2.7.15/lib/python2.7/site-packages/pkg_resources/__init__.py", line 3148, in parse
1933    req, = parse_requirements(s)
1934  File "/home/travis/virtualenv/python2.7.15/lib/python2.7/site-packages/pkg_resources/__init__.py", line 3094, in parse_requirements
1935    yield Requirement(line)
1936  File "/home/travis/virtualenv/python2.7.15/lib/python2.7/site-packages/pkg_resources/__init__.py", line 3103, in __init__
1937    raise RequirementParseError(str(e))
1938RequirementParseError: Invalid requirement, parse error at "',<5.0.0'"
1939The command "bin/buildout $VERSIONS" failed and exited with 1 during .
1940
1941

So somewhere buildout seems to get a requirement with an empty package name: ',<5.0.0'.
Tried locally, but no problem.
I have no idea

@fredvd
Copy link
Member

fredvd commented Sep 14, 2020

@idgserpro Are you using collective.cover latest versions in production in projects? In this test only the Plone 5.1 build is failing with an obscure version dependency isse where something ',<5.0.0'. We can look into this, but need cover at the moment for a Plone 4.3 site.

@idgserpro
Copy link
Member

@fredvd I think it might have to do with Travis' Python setuptools version. Travis configuration uses Python of Travis:

- python bootstrap.py

Perhaps installing the buildout with the pip this problem will be solved.

@mauritsvanrees
Copy link
Member Author

I have rebased on the branch from PR #882 and force pushed. That should fix Travis.

Changing the order in a list tile on the Compose tab would seemingly work at first, but nothing was saved.
The wrong element was taken as base (the whole html document instead of the tile).
And iterating over the children to get their uuids was done wrong, trying to take a uuid attribute from always that same wrong element.
@mauritsvanrees
Copy link
Member Author

Rebased on master and force pushed for the Travis/QA fixes.

@idgserpro
Copy link
Member

@mauritsvanrees I saw in the robot tests that checking the ordering of the items is done without reloading the page. See:

# move first item to the end
# Selenium doesn't seem to handle drag&drop correctly if the
# drop-target is not in the viewport. This code tries to work
# around the issue.
Execute Javascript window.scroll(0, 800)
Drag And Drop css=${first_item} css=${last_item}
Sleep 1s Wait for reordering to occur
# ensure that the reordering is reflected in the DOM
${first_item_title} = Get Text css=${first_item} h2
${last_item_title} = Get Text css=${last_item} h2
Should Be Equal ${first_item_title} My file
Should Be Equal ${last_item_title} My document
# first item is now last. Let's move it back to the top
Drag And Drop css=${last_item} css=${first_item}
Sleep 1s Wait for reordering to occur
# ensure that the reodering is reflected in the DOM
${first_item_title} = Get Text css=${first_item} h2
Should Be Equal ${first_item_title} My document

Perhaps you can reload the page before checking. This could be done in another PR, to confirm that the tests will fail, without fix. Then you can rebase here, to verify that the test will pass.

@mauritsvanrees
Copy link
Member Author

I see that on Plone 5 the robot tests are completely skipped, which is already the case since 2016. When I try them, it quickly fails with BadRequest: The id "title" is invalid - it is already in use..

And with Plone 4.3 I cannot run robot tests, because the remote webdrivers I have available are not compatible with robot framework and friends.

So I cannot really edit robot tests.

@fredvd
Copy link
Member

fredvd commented Sep 16, 2020

@idgserpro @mauritsvanrees So to summarise: the robot tests on Plone 4 can no longer work because the selenium webdrivers are no longer functionning and on Plone 5 the robot tests were always skipped already....

I have verified the correct working of the list tile fix from Maurits on Plone 4. @idgserpro: This was why I asked previously in this thread if you are supporting collective.cover in production installations at the moment. Are these Plone 4 or also Plone 5.0/5.1? Then we could verify the working of the list/ and related tiles manually and merge this fix.

I have the impression that collective.cover is just like Plone 4 end of life. We still have to support it on Plone 4 for 1 - 1.5 years but then our maintenance interestes will also be over. We don't have the resources at the moment to fix the robot test infrastructure just to merge this pull request.

Otherwise we'll just fork and keep our own maintenance branch

Copy link
Member

@hvelarde hvelarde left a comment

Choose a reason for hiding this comment

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

LGTM

@hvelarde
Copy link
Member

RF tests are running and passing on Travis as you can see here: https://travis-ci.org/github/collective/collective.cover/jobs/727634332

IIRC, there was a problem on supporting both Plone 4 and 5 with RF tests at the same time.

I think the testing matrix can be now simplified and at some point the RF tests must be migrated to support Plone 5.2 only.

collective.cover has been Plone 5.1 compatible since December last year.

@hvelarde hvelarde merged commit 1238065 into master Sep 16, 2020
@hvelarde hvelarde deleted the maurits/reorder-list-tile branch September 16, 2020 17:10
@mauritsvanrees
Copy link
Member Author

For the record: the problem I have with robot tests on Plone 4.3 is not specific to collective.cover. I can't get them to run for the Plone coredev buildout either. I have not tried in a while.

@idgserpro
Copy link
Member

@mauritsvanrees I managed to get test robots to work on Plone 4.3, with newer versions of Firefox. See:

plonegovbr/brasil.gov.agenda@cbbc7be

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants