After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 722825 - Update the recent place added time on re-visit
Update the recent place added time on re-visit
Status: RESOLVED FIXED
Product: gnome-maps
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-maps-maint
gnome-maps-maint
Depends on:
Blocks:
 
 
Reported: 2014-01-23 12:06 UTC by Jonas Danielsson
Modified: 2014-01-24 11:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
placeStore: sort store by ascending added time (1.22 KB, patch)
2014-01-23 12:07 UTC, Jonas Danielsson
committed Details | Review
placeStore: do not throw exception in store (1.11 KB, patch)
2014-01-23 12:07 UTC, Jonas Danielsson
committed Details | Review
Update recent added time on re-visit (1.95 KB, patch)
2014-01-23 12:07 UTC, Jonas Danielsson
committed Details | Review

Description Jonas Danielsson 2014-01-23 12:06:29 UTC
Right now we have a limit on the number of recent visited places. When that limit is hit we remove the oldest place.

We do not update the time of visit of a place if the place is re-visited. So when the recent places limit is hit the place removed might be a place you visited very recently.

I will post a patch series that keeps placeStore sorted by added time and makes sure we update the added time when a place is re-visited.
Comment 1 Jonas Danielsson 2014-01-23 12:07:39 UTC
Created attachment 267034 [details] [review]
placeStore: sort store by ascending added time
Comment 2 Jonas Danielsson 2014-01-23 12:07:42 UTC
Created attachment 267035 [details] [review]
placeStore: do not throw exception in store
Comment 3 Jonas Danielsson 2014-01-23 12:07:46 UTC
Created attachment 267036 [details] [review]
Update recent added time on re-visit
Comment 4 Zeeshan Ali 2014-01-23 12:55:42 UTC
Review of attachment 267034 [details] [review]:

ack
Comment 5 Zeeshan Ali 2014-01-23 13:00:00 UTC
Review of attachment 267035 [details] [review]:

ack
Comment 6 Zeeshan Ali 2014-01-23 13:08:11 UTC
Review of attachment 267036 [details] [review]:

::: src/placeStore.js
@@ +208,3 @@
+    },
+
+    _updateAddTime: function(place) {

Would go for: _updateAddedTime instead but up to you

@@ +216,3 @@
+                model.set_value(iter, Columns.ADDED, updated);
+                this._dirty = true;
+                this.store();

btw, I don't think we need this _dirty. We seem to be always setting it before calling store() and its only use is that store() ensures its true. Oh and store() doesn't unset it, which I imagine it should (if we keep _dirty for some reason).
Comment 7 Zeeshan Ali 2014-01-23 13:12:12 UTC
Review of attachment 267036 [details] [review]:

::: src/placeStore.js
@@ +216,3 @@
+                model.set_value(iter, Columns.ADDED, updated);
+                this._dirty = true;
+                this.store();

I actually removed it already in git master. Feel free to reintroduce it if its needed.
Comment 8 Jonas Danielsson 2014-01-24 11:28:35 UTC
Attachment 267034 [details] pushed as 8fcb33d - placeStore: sort store by ascending added time
Attachment 267035 [details] pushed as fcf7d96 - placeStore: do not throw exception in store
Attachment 267036 [details] pushed as accbfcf - Update recent added time on re-visit