openerp-expert-framework team mailing list archive

["Olivier Dony \(OpenERP\)" <odo@xxxxxxxxxxx>]

@Framework list: anyone have reasons to prefer keeping the current semantics of
ignoring empty CSV cells rather than treating them as NULL during CSV import?
See below and see also the complete merge proposal discussion for more details.
Thanks!


On 10/08/2012 03:44 PM, Xavier (Open ERP) wrote:
>> test_datetime should cover it, I don't think it checks what was imported.
> 
> It doesn't, nor does it provide various timezones to see what happens.
> 
> On the other hand, this begs the question: what happens if no timezone is
> provided, e.g. if the import is done through the API without specifying a
> context?

You're right, treating the values as UTC might not be the expected result.
What we do for the context lang when we think the context may not have it (e.g.
in workflows) is to use the `context_lang` (now just `lang`) stored for the
user performing the operation. It could make sense to do the same here.
And if the user has no timezone set, then we fallback to UTC anyhow.


>> I didn't see in the code / documentation any mention about how we could
>> empty a already defined value.
> 
> These fields are skipped at the moment, I kept the original semantics (so's
> not to have to fix files in some localization modules)
> 
>> In my opinion, it would be a shame to work on the imports without taking
>> some action for this issue.
> 
> That's a good point. The issue is that any value other than leaving the cell
> empty likely already means something (I mean unrealistically we could use a
> UUID for one or the other, but that'd be quite horrible), so empty can mean
> either "don't touch it" or "set to empty". I don't have any hard feelings
> one way or the other, I'd suggest discussing this on whatever mailing list
> you see fit and then I can change the semantics of the "empty field"
> whichever way was picked? Would that work?

I agree that it makes sense to have a way to empty a field using a CSV import,
thanks for pointing that out Guewen!
And I also agree that the only sensible choice for doing that is to have empty
cells mean NULL, no black magic.

Now I'm not sure why the current semantics is to skip them instead... perhaps
in order to support "sparse" updates?
The reference for CSV files being spreadsheets, I don't think users will expect
OpenERP to skip empty cells, so it sounds a sensible choice to have empty cells
mean NULL always. It would also stay compatible with the export, which is our
biggest constraint for all things import/export.

I requested an extra review from the framework team, to get more feedback on
this change.


>> the xid column header of the ir.model.data list view shows "Complete ID",
>> while users will presumably be looking for an "External ID"
> 
> Yeah that's the name it has on ir.model.data, and ir.model.data already uses
> "external identifier" for "name" (the xid without the module), not sure what
> to do about this.

Perhaps we could re-label the `complete_name` column to "External Identifier
(full)" and override it locally in that view to just "External Identifier" for
brevity?

-- 
https://code.launchpad.net/~openerp-dev/openobject-server/trunk-newimport-phase2-xmo/+merge/126658
Your team OpenERP Framework Experts is requested to review the proposed merge of lp:~openerp-dev/openobject-server/trunk-newimport-phase2-xmo into lp:openobject-server.