search

collective / collective.nitf

A Dexterity-based content type inspired on the News Industry Text Format specification
8 stars 3 forks source link

Fixes textIndexer for not breaking on empty fields #37

Closed jsbueno closed 11 years ago

jsbueno commented 11 years ago

The latest change by Hvelarde would break when any field was None - even a not required field.

This one line change fixes that replacing the Nonevalue with u"" before joining the field contents to compose the index.

hvelarde commented 11 years ago

you are fixing the effect but not the cause; see bcb21d321f3f61e9fbd20fc566ddb5ad1b869017

I would prefer that you write a test showing that an object with all optional fields empty, does not have any field returning None.

jsbueno commented 11 years ago

Should it bet forbidden to leave a field as "None" ? It worked otherwise just up to the change in this method - and changing the code I am developing that puts "None" there instead of an empty string is the easy part. The test you are requesting nonetheless implies that it would be forbidden to set any field to None - why should this, and only this method break that?

And even if fields should not contain None, there is no reason not to make the suggested change in the code, as it would avoid breaking already existing code that does so.

hvelarde commented 11 years ago

I made additional changes in c857b7983bae2baeec6b3a47fc1b46c18bb9af12; now it's working on a cleaner way and no further tests are needed I guess