Closed
Bug 952310
Opened 9 years ago
Closed 9 years ago
Create database schema for HomeProvider
Categories
(Firefox for Android Graveyard :: Data Providers, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: Margaret, Assigned: Margaret)
References
Details
(Whiteboard: shovel-ready)
Attachments
(2 files, 1 obsolete file)
4.62 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
13.10 KB,
patch
|
lucasr
:
review+
|
Details | Diff | Splinter Review |
Currently database use is disabled in HomeListsContentProvider until we can come up with a v1 schema that we're happy with. There's some discussion of schema in bug 941318, let's continue that discussion here.
Updated•9 years ago
|
Hardware: ARM → All
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → margaret.leibovic
Assignee | ||
Comment 1•9 years ago
|
||
As part of this, we should make the current HomeListItems columns more generic. E.g., instead of "title" and "url", we should have "primary_text" and "secondary_text" (or something like that).
Assignee | ||
Comment 2•9 years ago
|
||
There's a few different moving parts here, but we can use this bug to make sure we're representing the same schema in JS and Java.
Assignee | ||
Comment 3•9 years ago
|
||
Also building this on top of my patch for bug 949039, which should be ready to land soon. We'll want the fake items to match the same schema anyway.
Depends on: 949039
Assignee | ||
Comment 4•9 years ago
|
||
This is built on top of patches in the dependent bugs. If this is too annoying to review right now, you can also wait until those patches land (hopefully soon!).
Attachment #8364785 -
Flags: review?(lucasr.at.mozilla)
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8364785 [details] [diff] [review] (Part 1) Rename HomeListsProvider -> HomeProvider Review of attachment 8364785 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/robocop.ini @@ +37,5 @@ > skip-if = processor == "x86" > [testInputUrlBar] > [testJarReader] > [testLinkContextMenu] > +# [testHomeListsProvider] # see bug 952310 I decided to punt on updating testHomeListsProvider for now, since I want to figure out how to test querying the content provider from Java, while populating the database from JS... but maybe that's too ambitious and we should just insert test data from Java as well. I can address this in a later patch in this bug.
Assignee | ||
Comment 6•9 years ago
|
||
This patch has some more renaming, but it's main objective is to get things into the same state everywhere. Once again, this might be annoying to review before the dependent patches land :/
Attachment #8364788 -
Flags: review?(lucasr.at.mozilla)
Comment 7•9 years ago
|
||
Comment on attachment 8364788 [details] [diff] [review] (Part 2) Unify JS and Java representations of HomeProvider schema Review of attachment 8364788 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/db/HomeProvider.java @@ +116,3 @@ > item.getString("url"), > + item.getString("primary_text"), > + item.getString("secondary_text") I'm sorta coming back around to calling these title/description, to make it clear that one should be short and one can be long. Sorta matches how Android settings look... Meh. I also hate getting distracted on naming.
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #7) > Comment on attachment 8364788 [details] [diff] [review] > (Part 2) Unify JS and Java representations of HomeProvider schema > > Review of attachment 8364788 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/db/HomeProvider.java > @@ +116,3 @@ > > item.getString("url"), > > + item.getString("primary_text"), > > + item.getString("secondary_text") > > I'm sorta coming back around to calling these title/description, to make it > clear that one should be short and one can be long. Sorta matches how > Android settings look... > > Meh. I also hate getting distracted on naming. I don't think it's a distraction, since this is an API that will be exposed to developers. Calling these things title/description to match Android conventions sounds like a reasonable proposal. I think we just need to make sure we don't call them title/url, like we currently do, since right now our UI always assumes list items correspond to pages. (However, we should still have a url field that corresponds to an action, just not a user-visible thing.)
Assignee | ||
Comment 9•9 years ago
|
||
This updates the schema to match what we talked about this morning. I also verified locally that testHomeProvider still passes.
Attachment #8364788 -
Attachment is obsolete: true
Attachment #8364788 -
Flags: review?(lucasr.at.mozilla)
Attachment #8366129 -
Flags: review?(lucasr.at.mozilla)
Comment 10•9 years ago
|
||
Comment on attachment 8364785 [details] [diff] [review] (Part 1) Rename HomeListsProvider -> HomeProvider Review of attachment 8364785 [details] [diff] [review]: ----------------------------------------------------------------- Nice.
Attachment #8364785 -
Flags: review?(lucasr.at.mozilla) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8366129 [details] [diff] [review] (Part 2) Schema update Review of attachment 8366129 [details] [diff] [review]: ----------------------------------------------------------------- Cool.
Attachment #8366129 -
Flags: review?(lucasr.at.mozilla) → review+
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f88683073c1e https://hg.mozilla.org/integration/fx-team/rev/629b52808ae1
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f88683073c1e https://hg.mozilla.org/mozilla-central/rev/629b52808ae1
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•