499792 - new identity.js persist code passing wrong authorId value to persistPlace

Status: RESOLVED FIXED

(Mozilla Labs :: Snowl, defect)

Description

[alta88]

with changeset 50defdbb84d3, it seems SnowlPerson.persist is now creating the places record, but the places record needs to have the authorID be the identity table id matching to the message authorId, not the people table id (personID).

the problem may have arisen bacause personID was originally misnamed in SnowlIdentity.create, ie it was really the identity.id not the people.id.

so author/message removal isn't working.  i'm not (still alas) clear on the relationships here, so am not sure how to best fix in the flow etc.  the placeID is being stored correctly in people.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

I'm looking at this now, but I'm not entirely sure what the issue is.  I'm able to delete messages, and the code looks like it would remove a places record correctly when the last message from a given person is deleted.  Can you elaborate on the issue?

Comment 2

[alta88]

if you subscribe a new source, and then try to remove author, it won't be able to find by the right messages table id.  if you rt click Properties on an author row, you'll see the a.id= value (was authorID= prior to latest pre build) which should correspond to the identities.id (if i'm not mistaken).  author values stored prior to this are ok, it's new ones.

i recall in my db the people.id used to be the same as identities.id, but with 800 authors now, they aren't anymore (dont' know if this matters).  also, my dev db could have been corrupted etc etc but this should be, and isn't, corrected on a db rebuild.

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Hmm, here's the testcase I'm using and the results I get:

1. close Firefox, remove places.sqlite and messages.sqlite from the profile, and open Firefox;
2. go to http://www.melez.com/mykzilla/ and press the feed button in the location bar;
3. open the list view (Tools > Snowl > List);
4. open the All Authors treeitem, context-click the Myk author, and select Remove Author from the context menu.

When I do this, the Myk author disappears from the All Authors treeitem and All Sources > mykzilla no longer contains any messages.  Also, if I query the database I find that there are no records in the messages, identities, or people tables.

Is something else supposed to happen?

Note: identity and person IDs may be identical, but this is incidental.  Since a person can have multiple identities, there is no constraint that people and identities records have identical (or even related) IDs.

Perhaps I'm not seeing the problem because in my testcase the person and identity records happen to have the same ID?

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: treats a.id in Places URL as person ID

I added a person before subscribing to mykzilla to make sure identity/person IDs didn't match and was then able to reproduce the bug.  Here's a potential fix.  This fix treats the a.id value in Places URLs for people as person IDs rather than identity IDs.

This is the right thing to do, I think, as Places should be tracking people, not identities (even though the underlying data model links message records to the people records representing their authors via identity records).  People are the first-class object that we should be exposing to users.

Take a look and let me know if you can think of any problem with this approach.

Comment 5

[alta88]

ah, so if it's people.id that should be exposed, then that would change some other things that depend on a.id matching messages.authorID directly, like mark as read and the stats stuff recently checked in, plus the places db build/rebuild code.

so i'll figure out how to fix the places db rebuild function, as it's based on the original collections grouping code, and i need to rewrite it anyway as it's too slow for building places, and it looks like everything is converting in bits to collection2.

actually, i can do the other stuff too rather that you having to dig around.  should i assume that there will always be an identities.id for each people.id?  ie the same semantic person across 2 sources would have 2 people and 2 identities records?  this part of the model is unclear, especially because one would want those grouped/equated somehow, based on (unique) externalID automatically or else a user deciding they're the same and doing it manually.

(as a side thing, i think author is more correct than people, as an author may mean a person/human or organization or any 'publisher', while people only means the first.)

Comment 6

[Myk Melez [:myk] [@mykmelez]]

> should i assume that there will always be an identities.id for each people.id? 
> ie the same semantic person across 2 sources would have 2 people and 2
> identities records?

The same person across two sources would have one person and two identities records.  There should always be a person record for every identity record, since an identity is just a way of representing a person (with respect to a given source).  There might always be at least one identity record for every person record, although that's less clear.

Comment 7

[alta88]

so then if author is Joe in Source1 and Source2 with no unique externalID then 2 people records are created; if Joe is [e-mail] in Source1 and Source2 then the identities.personID would be the people.id created when Source1 was added.  does it work this way now?  i seem to have identical unique externalID values in multiple identities records and each has a different personID/people.id.

now that i get it, a bit of work to fix up authors..

Comment 8

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #7)
> so then if author is Joe in Source1 and Source2 with no unique externalID then
> 2 people records are created; if Joe is [e-mail] in Source1 and Source2
> then the identities.personID would be the people.id created when Source1 was
> added.  does it work this way now?

That's the way it should work, but it doesn't work that way yet, because there isn't yet code for determining that two identities represent the same person (neither programmatically nor via some user interface affordance for the user to tell us).

I've checked in the patch in changeset http://hg.mozilla.org/labs/snowl/rev/72bd213001e7.  I think that fixes this bug, but please reopen if there's still more to do.

Comment 9

[alta88]

a bunch of other issues fixed in
http://hg.mozdev.org/snowl/rev/6bf76e98cc04
given the people<->identities relationship.

actually it's likely not possible to auto associate identities across sources to a people record; lots of identical [e-mail] type ids, so short of some (pretty unlikely) universal enforceable person guid..