1274179 - Store table of contents pages in Firebase

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Ed Lee :Mardak]

Should be similar to adding text chat messages where the actual message is mostly opaque except the type is `tocPage`? We'll want to have actions to indicate the addition.

Comment 1

[Ed Lee :Mardak]

Bug 1273501 is to store the image thumbnail that should just be part of the opaque value.

Comment 2

[GitHub Autolander Bot]

Attachment: [loop] fzzzy:1274179-toc-in-firebase > mozilla:akita

Comment 3

[Ed Lee :Mardak]

Comment on attachment 8759171 [details] [review]
[loop] fzzzy:1274179-toc-in-firebase > mozilla:akita

Main thing is figuring out what data we actually want to store as a tile is more than just a single string. We don't quite need to figure out the data fields completely for this bug as it can be changed in bug 1273506, but it should at least be correct and make sense.

How about instead of a Tile containing string `tile`..

ContentTile:
  title: String
  url: String

ianb might have more thoughts?

Comment 4

[Ed Lee :Mardak]

Or maybe ContentPage instead of ContentTile?

Comment 5

[Ian Bicking (:ianbicking)]

There's a suggestion here: https://docs.google.com/document/d/1Rk0dwO6yrjmmwO3JU1gs4MwoNa9v-mecEoOGfjq-L4o/edit#

	id (string, the UUID)
	created (timestamp)
	modified (timestamp)
	addedById (Participant ID)
	url (string)
	canonicalUrl (string, best guess at the canonical URL, og:url, rel=”canonical”, etc)
	docTitle (string, from <title>)
	openGraph (map/object of og: properties)
	twitterCard (map/obect of twitter: properties)
	meta (map/object of <meta> properties)
	getter for .title (uses openGraph.title, docTitle)
	images (set of UUID → Image)

BUT, it is probably best to stick with what PageMetadata returns for now: https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/PageMetadata.jsm – and simply be clear about where the data comes from, so in the future if we add or change the properties we can still support the old metadata.  I'd recommend an object that wraps the JSON that makes up the page, and then that object can provide a consistent interface for new features while also supporting older versions.

Comment 6

[Ed Lee :Mardak]

Comment on attachment 8759171 [details] [review]
[loop] fzzzy:1274179-toc-in-firebase > mozilla:akita

r=Mardak with the last change

Comment 7

[Ed Lee :Mardak]

This is the third commit in a row that you've pushed without adding the reviewer in the commit message and linking to the commit to resolve the bug.

https://github.com/mozilla/loop/commit/795025285638a9df49a4e0ee7c6364d0ea4f0943

Comment 8

[Donovan Preston [:fzzzy]]

Each time, I was just following the instructions here:

https://github.com/mozilla/loop/blob/master/CONTRIBUTING.md#landing

Each time, I have gotten it wrong. The instructions need to be better. It was my own mistake for not knowing to use reword instead of pick while editing the commit message to add the reviewer; I did add the reviewer every single time. Now that Mark helped me, I know. The docs don't even mention linking to the commit at all.

Comment 9

[Ed Lee :Mardak]

(In reply to Donovan Preston [:fzzzy] from comment #8)
> The instructions need to be better.
True. It would help get them fixed sooner if you filed a bug or just replied with a simple comment after running into problems the first, second, or third time. It's hard for people to help if things sound fine.

> The docs don't even mention linking to the commit at all.
It does... in the autolander section, but that's been quite broken for a while now. ;)