fix for #53 #56

petschki · 2018-07-09T10:33:17Z

and some features:

uninstall profile
CSRF warning when editing taxonomy data the first time (write on read)
update/fix german translations

and some features: - uninstall profile - CSRF warning when editing taxonomy data the first time (write on read) - update/fix german translations

petschki · 2018-07-09T10:59:58Z

what's the reason why travis doesn't start?

hvelarde · 2018-07-09T21:08:37Z

src/collective/taxonomy/jsonimpl.py

@@ -24,7 +29,7 @@ def __init__(self, context, request):
        utility_name = request.get('taxonomy', '')
        taxonomy = queryUtility(ITaxonomy, name=utility_name)
        if not taxonomy:
-            raise ValueError('Taxonomy `%s` could not be found.' % utility_name)
+            raise ValueError('Taxonomy `%s` could not be found.' % utility_name)  # noqa


please don't use generic noqa comments; it's better to explicitly specify what are you trying to ignore.

make code-analysis happy

malthe · 2018-07-10T07:13:32Z

src/collective/taxonomy/exportimport.py

                    except ConfigParser.NoOptionError:
                        pass

                for name in ['is_single_select', 'is_required']:
                    try:
-                        result[name] = config.get('taxonomy', name) == 'true' and True
+                        result[name] = config.get('taxonomy', name) == 'true' and True  # noqa: E501


The and True here seems to be reminiscent of a time when there wasn't an explicit comparison involved – as far as I can tell it's unnecessary.

you're right ... I'll change that

malthe · 2018-07-10T07:16:29Z

src/collective/taxonomy/jsonimpl.py

@@ -47,6 +52,7 @@ def generate_json(self, root):

    def get_data(self):
        """Get json data."""
+        alsoProvides(self.request, IDisableCSRFProtection)


Why is this necessary here? No ZODB object seems to be modified in this code.

I have do dig here further, but if you have a clean Plone-5.1 installation, create a taxonomy and want to edit data for the first time, you get a CSRF warning from plone.protect ...

Maybe it happens in the _fixup method.

You could argue that there shouldn't be any fixing upping on a GET request.

thanks for the pointer ... the _fixup was indeed the problem ... I've refactored this to get rid of CSRF warnings ...

The dilemma is this:

There's an old version of the data format and there is an upgrade step which migrates this to the new format by calling _fixup().

But what if people try to use their site without – yet – having gone through the upgrade step.

I think that was the reasoning behind these fixing upper calls. And there used to be no upgrade step!

But now, perhaps the right move is simply to throw and error and log a warning "Data not upgraded" – ?

That is:

I think revert that last commit;

In _fixup(): if said data attributes are None, add the CSRF interface to the request and just go ahead and migrate the data.

This reverts commit 8c7dd0b.

malthe · 2018-07-10T09:39:16Z

src/collective/taxonomy/utility.py

+        # for old Taxonomy instances.
+        # XXX: remove this in version 2.0 to prevent write on read
+        if self.order is None:
+            safeWrite(self, getRequest())


Great. Poor old self.REQUEST – acquisition has fallen out of favor...

strange .. I got an AttributeError when doing self.REQUEST in _fixup ?

I guess the utility is not acquisition-wrapped. Makes sense I guess.

Plone is really split brain in terms of acquisition and components.

malthe · 2018-07-10T10:15:56Z

I guess we're ready to merge this then?

petschki · 2018-07-10T10:17:08Z

yes ... and I also think we should release 1.5.0 ... changelog gets longer and longer 😉

petschki · 2018-07-10T10:19:01Z

@malthe but before I'd like to run the tests ... somehow my branch doesn't start automatically on travis-ci .... do you have an idea why?

malthe · 2018-07-10T10:40:51Z

I think the buildout is outdated.

While:
  Installing.
  Getting section download.
  Initializing section download.
  Installing recipe hexagonit.recipe.download.
  Getting distribution for 'hexagonit.recipe.download'.
Error: Wheels are not supported
The command "bin/buildout -N -q versions:setuptools= versions:zc.buildout=" failed and exited with 1 during .

petschki · 2018-07-10T11:28:35Z

I thought there would be some errors with this travis configuration, but this branch never ever gets started on travis-ci ... https://travis-ci.org/collective/collective.taxonomy says "This is not an active repository" .... I do not have privileges to activate it. Do you have @malthe ?

malthe · 2018-07-10T11:53:48Z

I don't really understand if I'm an admin or not. I can use "Settings" on that repo, but I also get the same error message.

petschki · 2018-07-10T11:55:10Z

just found this entry from june 29th stating some issues with de/activating repositories on https://www.traviscistatus.com/ ... I'll get in touch with travis support

petschki · 2018-07-10T12:13:37Z

ping @jensens are you able to activate this repository on travis-ci, or do you have an idea what's going wrong here? https://travis-ci.org/collective/collective.taxonomy
thx

jensens · 2018-07-10T12:49:05Z

I activated it on TravisCI (.org) - no idea why it was deactivated there.

petschki · 2018-07-10T14:05:28Z

@malthe finally

fix for #53

70d872d

and some features: - uninstall profile - CSRF warning when editing taxonomy data the first time (write on read) - update/fix german translations

petschki requested a review from tomgross July 9, 2018 10:34

fix coverage script on travis

05543fd

petschki requested a review from malthe July 9, 2018 10:59

hvelarde reviewed Jul 9, 2018

View reviewed changes

pep8

989cfb5

make code-analysis happy

malthe reviewed Jul 10, 2018

View reviewed changes

petschki added 3 commits July 10, 2018 10:18

fix CSRF warnings due to _fixup method (write on read)

8c7dd0b

Revert "fix CSRF warnings due to _fixup method (write on read)"

95fefbd

This reverts commit 8c7dd0b.

fix compatibility issues with old taxonomy code

14fd8b3

malthe reviewed Jul 10, 2018

View reviewed changes

get rid of boostrap.py and fix travis configuration

3506ea9

petschki added 2 commits July 10, 2018 15:13

update travis config

5c623c1

fix import for plone.protect < 3.x

694adf5

malthe merged commit 923b6ca into master Jul 10, 2018

malthe deleted the petschki-fix_53 branch July 10, 2018 14:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix for #53 #56

fix for #53 #56

petschki commented Jul 9, 2018

petschki commented Jul 9, 2018

hvelarde Jul 9, 2018

petschki Jul 10, 2018

malthe Jul 10, 2018

petschki Jul 10, 2018

malthe Jul 10, 2018

petschki Jul 10, 2018

malthe Jul 10, 2018

petschki Jul 10, 2018

malthe Jul 10, 2018 •

edited

Loading

malthe Jul 10, 2018

petschki Jul 10, 2018

malthe Jul 10, 2018

malthe commented Jul 10, 2018

petschki commented Jul 10, 2018

petschki commented Jul 10, 2018

malthe commented Jul 10, 2018 •

edited

Loading

petschki commented Jul 10, 2018

malthe commented Jul 10, 2018

petschki commented Jul 10, 2018

petschki commented Jul 10, 2018

jensens commented Jul 10, 2018

petschki commented Jul 10, 2018

fix for #53 #56

fix for #53 #56

Conversation

petschki commented Jul 9, 2018

petschki commented Jul 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malthe Jul 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malthe commented Jul 10, 2018

petschki commented Jul 10, 2018

petschki commented Jul 10, 2018

malthe commented Jul 10, 2018 • edited Loading

petschki commented Jul 10, 2018

malthe commented Jul 10, 2018

petschki commented Jul 10, 2018

petschki commented Jul 10, 2018

jensens commented Jul 10, 2018

petschki commented Jul 10, 2018

malthe Jul 10, 2018 •

edited

Loading

malthe commented Jul 10, 2018 •

edited

Loading