GNOME Bugzilla – Bug 703703
Log Create, Delete, Modify and Leave for notes
Last modified: 2013-07-14 22:38:23 UTC
Created attachment 248507 [details] Screenshot of bijiben access event At the moment, bijiben only logs insertion of event, but it would be great to log the creation and deletion of events too, which can mark as history of notes and can further enhance bijiben. This event can be inserted after the titlebar of the event is known even though the note has been create way before. It isn't an issue since you can insert events in zeitgeist at an earlier date too. i.e. you can insert an event with timestamp a month ago. Snapshot provided. The application I am using for monitoring the events is called zeitgeist-explorer which is developed here https://github.com/zeitgeist-project/zeitgeist-explorer and available for download here https://launchpad.net/zeitgeist-explorer
Created attachment 248804 [details] [review] Adds zeitgeist LEAVE and DELETE event logging support I worked on adding LEAVE and DELETE event logging support. After this is acceptable, then I will work on CREATE event which would be slightly more tricky. (Better to submit smaller patches for easier review and regression checks)
Here is how it looks like on zeitgeist-explorer when logged http://i.imgur.com/3HT549Q.png
Comment on attachment 248804 [details] [review] Adds zeitgeist LEAVE and DELETE event logging support >--- a/src/libbiji/biji-note-obj.c >+++ b/src/libbiji/biji-note-obj.c >@@ -137,6 +137,8 @@ biji_note_obj_finalize (GObject *object) > { > BijiNoteObj *self = BIJI_NOTE_OBJ(object); > BijiNoteObjPrivate *priv = self->priv; >+ >+ insert_zeitgeist (self, ZEITGEIST_ZG_DELETE_EVENT); biji_note_obj_finalize only handles memory, note is actually deleted into biji_note_obj_trash. >@@ -853,6 +855,8 @@ _biji_note_obj_close (BijiNoteObj *note) > item = BIJI_ITEM (note); > book = biji_item_get_book (BIJI_ITEM (note)); > priv->editor = NULL; >+ >+ insert_zeitgeist (note, ZEITGEIST_ZG_LEAVE_EVENT); this one feels good Another concern is title changing. The note is opened once but title can change several times, should we amend the last ZEITGEIST_ACCESS_EVENT log when title changes?
Created attachment 248815 [details] [review] Update the patch for LEAVE and DELETE Fixed the patch by calling Delete event from inside trash function Secondly, an event already inserted cannot be amended. At max you can delete an event and insert another with same data with a previous timestamp If title can change several time, then it should not be a problem. If a title is present whenever an event if logged, then that title should be in text field. If there are MODIFY event logged for "save note" feature, then whatever the title is at that moment, that should be used. The trick is what to do when no title is present? A truncated body text?
Comment on attachment 248815 [details] [review] Update the patch for LEAVE and DELETE Thanks!, committed
(In reply to comment #4) > The trick is what to do when no title is present? A truncated body text? When a note is created, we could try wait for first save before logging the creation event. If the note remains blank and user changes his mind, note is deleted and nothing would. be logged. First save is triggered when there is some text, there should be a title (or, I should fix this). So the remaining work would be to detect, when a save happens, if the note was already created, and log if it wasn't.
>When a note is created, we could try wait for first save before logging the creation event Also wait for first ACCESS_EVENT too. When the first save happens, insert the first CREATE_EVENT and ACCESS_EVENT too, both with same timestamp back in time. The trick is how to figure out that no CREATE_EVENT/ACCESS_EVENT has been logged for a specific note? I am proposing a hack (maybe you can come with a better solution) is to have a create_timestamp with each note set in *_note_init When inserting SAVE_EVENT: * Insert SAVE_EVENT * Check if create_timestamp is set. - Clear create_timestamp field - If set use create_timestamp to insert CREATE_EVENT AND ACCESS_EVENT When inserting ACCESS_EVENT: * Check if create_timestamp is set. - If Check if create_timestamp is unset, then insert ACCESS_EVENT This avoids problems like * First ACCESS_EVENT with no title * Useless ACCESS_EVENT for empty notes which would be deleted * No logging of CREATE_EVENT at all when DELETE_EVENT is being logged. It is always good to have complementary events - CREATE AND DELETE, ACCESS and LEAVE.
Created attachment 248903 [details] [review] Adds GDateTime argument in insert_event for past events Proposing this patch which sets up basic structure for logging events in the past. The function insert_zeitgeist needs to know if the event is to be inserted in the past. Pass this argument as NULL to ignore. If not NULL, then the datetime will be explicitly set as the event timestamp
Comment on attachment 248903 [details] [review] Adds GDateTime argument in insert_event for past events > void insert_zeitgeist (BijiNoteObj *note, >- gchar *zg_interpretation); >+ gchar *zg_interpretation, >+ GDateTime *datetime); > The note stores a gint64, maybe can we use this? Otherwise, the patch seems fine. I propose to wait for some function to actually use the timestamp optional arg before to push it.
I wanted to submit the patch of this change plus the CREATE_EVENT together, then I decided that smaller patches are better to handle. Surely, I will post the second patch which would log CREATE_EVENT as I described in comment #7 This second patch would depend on patch #248903 Maybe both can be applied together.
The issue with using gint64 time is that the there is no way to know if the CREATE_EVENT has already been logged or not. With my proposed create_timestamp when this timestamp is set, it means that CREATE_TIMESTAMP has not been inserted. When cleared it means that it does not need to be inserted again. The gint64 field won't be unset, so there is no way to stop duplicate CREATE_EVENT. create_timestamp avoids duplicate CREATE_EVENT as better explained in comment #7
Created attachment 249061 [details] [review] Adds MODIFY_EVENT for SAVE and stops logging ACCESS_EVENT for newly created notes This note overrides the previous patch which would need to create a new create_timestamp with note. This patch logs MODIFY_EVENT for Save events and does not log ACCESS_EVENT for notes without a non-empty title Though the CREATE_EVENT event is not logged in this patch, it will be done next.For this I need to know which function to call if I have the URI of the note to get the BijiNoteObj instance
Review of attachment 249061 [details] [review]: This patch applies fine
You can find a pointer to the note if you have a pointer to the note book. Use biji_note_book_get_item_at_path. Check for the returned value. If there is a returned value, an "item", you can cast it to a note. Just use a path without "file://" prefix
Created attachment 249099 [details] [review] Patch to insert CREATE_EVENT This patch compliments and depends on patch #249061 When save is called, also try to check if CREATE_EVENT has been inserted or not. Ask the daemon for any CREATE_EVENT for this specific URI. If no event is returned, then it means there are no CREATE_EVENT inserted, so try to insert it. In the insert_zeitgeist event, check if it is a CREATE_EVENT, if yes, then set that specific event's timestamp to note's creation timestamp which will log the event in the past.
Created attachment 249143 [details] [review] Log modify delete and creation Cool! Seems it does the whole job now. I hope you're fine with minor style amendments - biji-zeitgeist header now only provides insert_zeitgeist. If the request log is a MODIFY_EVENT, then insert_zeitgeist func does the check_insert_create_zeitgeist call - the "title must not be null" check was slightly amended and is now done in insert_zeitgeist itself - so we're sure the check is done for any log
Yeah. I am fine with minor style amendments. Better to have most things in on function so that the flow of control is easier to understand.
Pushed, thanks