launchpad-dev team mailing list archive

[Guilherme Salgado <guilherme.salgado@xxxxxxxxxx>]

Hi,
(resending this as I wasn't subscribed to the list when I sent the first
time; I apologise in advance if you get this twice)

As we've mentioned earlier the new UI to edit work items is going to be
basically the same thing we currently have, that is, a text area with
one work item per line, with optional headings to target certain work
items to a milestone.  The main difference is that now we need to parse
that and create/update/delete SpecificationWorkItem objects for every row.

My plan was to have a read-only field (workitems_text), with a mutator
method (updateWorkItems(text)) and a custom Text field to do the
parsing/validation (which involves looking up assignees/milestones in
the DB to check that they exist/are valid).  However, I just realized
that for this to work I'd need my mutator method to look something like

    workitems_text = WorkItemsText(..., readonly=True)
    @mutator_for(workitems_text)
    def updateWorkItems(self, text):
        field = ISpecification['workitems_text'].bind(self)
        for workitem in field.parse(text):
            # Create new WI or update existing one

And this means the parsing (which implies validating the input as well)
will happen twice: once when the form machinery validates the input and
again when it passes the raw input to updateWorkItems(). Since the
parsing involves looking up objects in the DB, doing this twice is not
ideal.

One alternative I can think of is to make workitems_text a read-write
@property (marking it as read-write in the interface as well), with
updateWorkItems(wi_list) (notice that now it takes a list of pre-parsed
work-items) as its setter, and have the custom field store the parsed
value in an instance attribute that can later be assigned to
'workitems_text' (in the set() method of the field).  I really don't
like that because even though we're presenting 'workitems_text' as
something that looks like a Text field, you cannot assign any text to it
-- you'd have to assign a list of work items.

IMHO, the latter is not something we should do, so I'm starting to think
there's no way around having to parse the whole thing twice, but I
thought I'd ask in case anyone knows a better way to do this.

-- 
Guilherme Salgado <https://launchpad.net/~salgado>

Attachment: signature.asc
Description: OpenPGP digital signature