1208352 - Refactor places.js and history.js for extracting the similar logic of Places DataStore

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Sync, defect, P1)

Description

[Sean Lee [:seanlee][:weilonge]]

At bug 1191773 comment 51, PlacesDS accessing code should be extracted to a shared code to prevent drifting out of sync from different piece codes.
In my basic idea, the extraction work for editPlace#places.js and addPlace#history.js is the first aim.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] michielbdejong:1208352-shared-places-code > mozilla-b2g:master

Comment 2

[Michiel de Jong [:michielbdejong]]

Comment on attachment 8683644 [details] [review]
[gaia] michielbdejong:1208352-shared-places-code > mozilla-b2g:master

This is getting quite complex... Search, Homescreen and System all have their own funny way of accessing the places DataStore.

Also, the runtime 'verify' check is not what we want of course, we want the four apps to all use the same shared code.

Just wanted to ping you in case you have any advice on this. Should I make this a big refactor of all four apps or do you see an easier way out?

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Yes, the access to this DataStore is quite spread, that's why we need this refactor :) And if there's a moment to do this kind of refactors is now, right after branching 2.5.

I believe Dale worked on this, so he is the best one to give you feedback and guidance here.

Comment 4

[Dale Harvey (:daleharvey)]

Yeh this is looking mostly good to to me, I added a few notes on the PR, mostly aesthetic / minor things. and yeh the entire point was that if we have 4 apps touching that datastore, I would like them to do so in a consistent way (particularly as you put in there, the validation etc). I feel like the 'edit' logic should likely go in there as well at some point, but whats there looks like it would be a good start

Comment 5

[Michiel de Jong [:michielbdejong]]

Moving from apps/sync/js/adapters/history.js to shared/js/places_model.js:

_ensureStore
addPlace (except for setDataStoreId)
mergeRecordsToDataStore
deleteByDataStoreId
deletePlace
checkIfClearedSince

Comment 6

[Michiel de Jong [:michielbdejong]]

Comment on attachment 8683644 [details] [review]
[gaia] michielbdejong:1208352-shared-places-code > mozilla-b2g:master

Hi Sean,


I'm waiting for the other two commits to land before requesting review, but can you maybe give me some early feedback on https://github.com/michielbdejong/gaia/commit/bdbfd268c093cccc29a8800b3f407e8a7a8e3ae1 (third commit only)?

Comment 7

[BMO Automation]

Firefox OS is not being worked on