Need API to delete documents from QML

Bug #1243395 reported by Rick Spencer
20
This bug affects 4 people
Affects Status Importance Assigned to Milestone
U1DB Qt/ QML
Fix Released
High
Cris Dywan
u1db-qt (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

calls to delete_doc produce the following error:
TypeError: Object U1db::Database(0x13cd430) has no method 'delete_doc'

Related branches

Revision history for this message
Cris Dywan (kalikiana) wrote :

No surprise here because there's no such function. The accepted way of doing this is:

database.putDoc("", docId)

That said, I would be open to (a patch) adding a function for it to reduce confusion among those who feel more confident with a specific remove() function.

Changed in u1db-qt:
status: New → Invalid
Revision history for this message
Pat McGowan (pat-mcgowan) wrote :

 When you use the database as a model for lists, and if you set the document as null, it just adds an empty item to the listview or repeater

Re-opening the bug to get a solution at the qml level

Changed in u1db-qt:
status: Invalid → New
Revision history for this message
Cris Dywan (kalikiana) wrote :

There may be overlap with bug 1219862 but let's use this for the API - as I come to realize it would make bugs easier to distinguish if we had a function to talk about.

summary: - cannot delete documents from QML
+ Need API to delete documents from QML
Changed in u1db-qt:
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Christian Dywan (kalikiana)
Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

A suppress empty documents flag for Query or Index is a good idea.

Suppressing by default is probably going to be the preferable choice most of the time, but it is a good idea to still give the developer full control if they choose to use it.

Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

Having an empty document as a delegate of the ListView should be expected behaviour, if the document in the model is empty. However, a deleted document and empty document are not the same thing.

The original high level API documentation for U1DB states: "Mark a document as deleted. Will abort if the current revision doesn’t match doc.rev. This will also set doc.content to None."

It appears that in the reference implementation (in Python) a blank or empty document is still represented as an empty JSON string "{}", while a deleted document has contents that are stored as completly empty (aka the 'None' mentioned above).

There isn't anything else to "mark a document as deleted", and in fact the document still exists in the database -- it is never truly deleted or removed 100%.

The only difference between a (pseudo-)deleted document and a document with no content is that the former has lost the attribute 'content' at the Python level, while the latter still has that attribute (it is simply an empty JSON object).

Adding a boolean property to the Document class called "delete", set to false by default, which will empty the content in the underlying database (the SQLite database behind the scene), would be a very declarative approach to deleting documents.

When it comes to suppressing Documents with no content (aka "deleted"), it might not be possible to remove the 'contents' attribute at runtime. This leaves only checking if the contents are blank (but not simply an empty JSON string).

As for when and where those documents should be filtered out, i.e. in what class that functionality should exist, perhaps it is a matter of opinion, but it still seems to me that Query or Index are the appropriate places to filter out "deleted" documents and maybe even blank documents too, with an appropriate attribute. It may be the case that people want a list of deleted/blank documents, and removing the ability to get that information would then be undesireable.

Database could (and maybe should) have a default Query and Index pair, which the developer can access to modify the state for suppressing "deleted" documents (and again perhaps blank ones).

A dedicated UI component for use with U1Db-Qt could also offer some potential for complimenting the plugin itself, and offer another opportunity to add feature requests appropriate to the more declarative approach.

Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

"Adding a boolean property to the Document class called 'delete' "

'delete' should read 'deleted'

Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

Regarding https://bugs.launchpad.net/u1db-qt/+bug/1243395/comments/2, that sounds a little bit similar to doing this:

delegate: Item{
                width: 180; height: 50
                Text {
                    text: name + ": " + number
                }
                MouseArea {
                    anchors.fill: parent
                    onClicked:{parent.parent=null}
                }
            }

Which will also result in an empty item in a ListView, rather than deleting or removing it.

Assuming "contactmodel" is the model used by the ListView, something more like this will remove the item:

delegate: Item{
                width: 180; height: 50
                Text {
                    text: name + ": " + number
                }
                MouseArea {
                    anchors.fill: parent
                    onClicked:{contactmodel.remove(parent.parent.index)}
                }
            }

The Item needs to be removed from the model, not simply made null.

Revision history for this message
Cris Dywan (kalikiana) wrote :

> Having an empty document as a delegate of the ListView should be expected behaviour,
> if the document in the model is empty. However, a deleted document
> and empty document are not the same thing.

That's not in line with current API semantics: Query won't show "empty aka deleted" documents, there's bug 1219862 about making Database not show them. I'm a bit puzzled at the "brainstorming" of existing behavior that people use out there already.

> Regarding comment #2, that sounds a little bit similar to doing this:
> The Item needs to be removed from the model, not simply made null.

This is exactly what convinced me to propose API for this. I don't want to second-guess bugs. "Set to null" might refer to a variable, contents or putDoc. And even your suggestion "removed from the model" is not clear to me as a developer since your incomplete example uses a non-existing "remove" method.

Having Database.removeDoc(docId) would make this clear. Either it works or it doesn't. If we were to add a distinction between empty and deleted documents as you suggested above (which would be a behavior change!) the method could be updated to do it.

Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

The example snippets above for the definition of a delegate for a ListView use straightforward QML. It isn't specific to U1db-Qt. The remove method already exists in QML.

The first of the two was to explain that making a document==null, as suggested by one of the comments (that reopened the bug report) isn't likely to do what is seemingly intended.

The second of the two is an illustration of how to approach removing an item from a model of a ListView, using the remove method that already exists for that purpose in QML.

None of those comments or suggestions were questioning the validity of the bug report. Suggesting a property for Document called "deleted" that is set to true for false, or other suggestions, doesn't compete with or contradict a feature request for a delete_doc method.

Revision history for this message
Kevin Wright (kevin-wright-1) wrote :

>That's not in line with current API semantics:
>Query won't show "empty aka deleted" documents,

A Document that has 'contents' from the underlying data repository that are represented by an empty JSON string ("{}") and one that is completely empty ("") are not (supposed to be) the same thing. The former is intended to create a JSON object, the latter is not. The former is still (supposed to be) a document, the latter is supposed to be a 'deleted' document (aka tombstone).

Tombstone is actually the less misleading term, because no document is every really deleted. In the reference implementation only its content property is deleted. A tombstone document can still be retrieved from the database, but when trying to request its contents the user / developer will get an error that 'contents' does not exist as part of the obkext. (Or something to that effect).

Revision history for this message
Cris Dywan (kalikiana) wrote :

> The second of the two is an illustration of how to approach
> removing an item from a model of a ListView, using the
> remove method that already exists for that purpose in QML.

What kind of model? How would it remove documents from a U1Db database? I don't follow.

> A Document that has 'contents' from the underlying data repository
> that are represented by an empty JSON string ("{}") and
> one that is completely empty ("") are not (supposed to be) the same thing

There is no way to distinguish that in the API. putDoc('', docId) is the current way to get rid of documents and making them not show up in a ListView anymore. There're unit tests verifying this behavior.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:u1db-qt at revision 110, scheduled for release in u1db-qt, milestone 1.0

Changed in u1db-qt:
status: Confirmed → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package u1db-qt - 0.1.5+14.04.20140313-0ubuntu1

---------------
u1db-qt (0.1.5+14.04.20140313-0ubuntu1) trusty; urgency=low

  [ CI bot ]
  * No change rebuild against Qt 5.2.1.

  [ Christian Dywan ]
  * Adopt xvfb.sh script from ui toolkit to run tests
  * Sort out build warnings and make them always fatal.
  * Implement Database.removeDoc method and use it in unit test
    Functionally this is equivalent to replacing the doc with an empty
    one. (LP: #1243395)
  * Use new-style qmlrunner log option to enable stdout.
  * Query improvements and more advanced example. (LP: #1271977,
    #1271972, #1266478)
  * Store whole document contents in the results and unit test that.
    (LP: #1271973)
  * Reverse query logic to check non-matching and internally convert
    between query syntaxes. (LP: #1284194, #1214538, #1215898)
  * Revert r113 and update unit test to verify previous behavior
 -- Ubuntu daily release <email address hidden> Thu, 13 Mar 2014 23:12:40 +0000

Changed in u1db-qt (Ubuntu):
status: New → Fix Released
David Planella (dpm)
Changed in u1db-qt:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.