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 321770 - Keeping track of last 20 import sessions
Keeping track of last 20 import sessions
Status: RESOLVED FIXED
Product: f-spot
Classification: Other
Component: Editing
CVS
Other All
: Normal enhancement
: ---
Assigned To: Bengt Thuree
F-spot maintainers
: 328357 363941 (view as bug list)
Depends on:
Blocks: 342482
 
 
Reported: 2005-11-18 03:43 UTC by Bengt Thuree
Modified: 2007-05-24 20:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add the tag "Last Import" to imprted photos (2.96 KB, patch)
2006-01-24 09:22 UTC, Gunnar Steinn Magnusson
none Details | Review
Add 6 last imports as tags (6.27 KB, patch)
2006-05-19 09:01 UTC, Bengt Thuree
none Details | Review
Menu screenshot (8.40 KB, image/png)
2006-05-31 06:37 UTC, Bengt Thuree
  Details
Last Roll Filter gui (50.55 KB, image/png)
2006-05-31 06:38 UTC, Bengt Thuree
  Details
Last Roll Query (55.54 KB, image/png)
2006-05-31 06:40 UTC, Bengt Thuree
  Details
Patch (65.88 KB, patch)
2006-05-31 06:51 UTC, Bengt Thuree
none Details | Review
Updated patch (66.85 KB, patch)
2006-05-31 15:24 UTC, Bengt Thuree
none Details | Review
updated patch against CVS (62.19 KB, patch)
2006-06-07 09:53 UTC, Bengt Thuree
none Details | Review
updated updated patch against CVS (69.45 KB, patch)
2006-06-07 10:41 UTC, Stephane Delcroix
none Details | Review
Double updated patch against cvs (69.45 KB, patch)
2006-06-07 11:03 UTC, Bengt Thuree
none Details | Review
Tripple updated patch against cvs (70.18 KB, patch)
2006-06-08 00:53 UTC, Bengt Thuree
none Details | Review
Patch to apply after applying 342137 (67.24 KB, patch)
2006-07-22 09:35 UTC, Bengt Thuree
none Details | Review
Bengst Patch against CVS (53.33 KB, patch)
2006-08-03 14:07 UTC, Bengt Thuree
none Details | Review
Patch with permanent import session in photo (59.89 KB, patch)
2006-09-26 14:20 UTC, Bengt Thuree
needs-work Details | Review
Patch with permanent roll id in photo (59.57 KB, patch)
2006-10-01 13:11 UTC, Bengt Thuree
none Details | Review
Patch with permanent roll id in photo (158.96 KB, patch)
2007-04-19 14:49 UTC, Bengt Thuree
none Details | Review
Patch with permanent roll id in photo (50.33 KB, patch)
2007-04-19 14:55 UTC, Bengt Thuree
none Details | Review
Patch with permanent roll id in photo (52.01 KB, patch)
2007-04-19 15:36 UTC, Bengt Thuree
none Details | Review
reworked patch (45.66 KB, patch)
2007-05-23 17:36 UTC, Stephane Delcroix
committed Details | Review

Description Bengt Thuree 2005-11-18 03:43:51 UTC
Would also be good to tag the imported pictures with a specific
import-session tag, so you can select all just imported pictures and
only work/view those. As well as previous imported batches (say last
10-30 batches or so). Much easier to work with only 30 pictures than
10.000 when you are setting the tags, renaming pictures etc, adding
titles and other informations etc.

And it would be good to be able to check the last, say 10-30, imported sessions,
since you might have done a few import batches.
Comment 1 Bengt Thuree 2006-01-10 06:33:24 UTC
Lightroom (http://labs.macromedia.com/technologies/lightroom/) have this feature of selecting and working only with the latest imported photos.
I think just the latest might not be enough, but the last 5-10 sessions?
Importing 3 different memory cards for instance as I am doing right now.
Comment 2 Bengt Thuree 2006-01-10 06:44:19 UTC
How about possibility to set the most common EXIF/XMP tags directly during the import?
For instance : 
  Copyright
  Photographer
  Country
  Location
  Town
  Header/Comment 
  a number of tags
Comment 3 Gabriel Burt 2006-01-24 03:16:57 UTC
*** Bug 328357 has been marked as a duplicate of this bug. ***
Comment 4 Gunnar Steinn Magnusson 2006-01-24 09:22:17 UTC
Created attachment 57999 [details] [review]
Add the tag "Last Import" to imprted photos

Here is a patch I created for #328357 (I didn't find this bug before I posted that).
Basically it creates a tag called "Last Import" and on import it deletes the tag from all photos and adds it to the new ones.

I know it's not the "view last 10-20 batches" it is a start and would be enough in most use cases.

Regards, Gunnar Steinn
Comment 5 Tim LeRoy 2006-01-27 11:27:09 UTC
what about an "imported:" or "import session started:" field, or an "import sessions" table with the images linking to it?
Comment 6 Bengt Thuree 2006-01-29 03:50:54 UTC
Why not a simple 0-9 field, which automatically tags the imported pictures in this session with the last ImportSessionID (from preferences?) and increment it with 1. If it hits 10, change it to 0. 
This way we will be able to keep track of the last 10 sessions, and it should be easy for F-Spot to handle. Should not be neccessary to know the date we imported it. But that could also be tracked in a separate sqlite table. The ID (0-9), Date Imported, Number of Photos imported, UserID of importer for instance.

Today we can select ALL Pictures wihtout any tags. But as soon as we put one tag on a picture, we can not select it anymore.
How about we automatically tag a picture with a "Tags in progress" tag. So we can easily select all pictures we are in the process of tagging? And after we have finished tagging them (same day, or 2 weeks later) we remove the "Tags in progress" tag. 
Comment 7 Bengt Thuree 2006-05-18 08:48:13 UTC
I will have a look at this patch in the next day or so.
Comment 8 Bengt Thuree 2006-05-18 16:06:19 UTC
Brief check of patch for 321770
===============================

Check for ==> for things to modify.

==> Patch do not apply cleanly to CVS
Easy to manually fix though. 
(Observe "Catalog.GetString ("Last Import"))" instead of "Last Import".
This needs to be changed in TagStore.cs as well as in ImportCommand.cs)

I have imported a number of different batches, and it do seem to work nicely.

==> I checked the code a bit, and see that first there is a for loop to commit any new tag which the user have added during import. Then later on there is the new for loop with this latest import tag commit. Would it be possible to just have one commit per photo?
With this patch, you would always tag a photo, so the for loop should always be executed. Moving the check from before for loop to before setting the tag.

Am wondering if you have checked for possible slowdown due to the extra commits?
==> Have you tested how long time it takes to import 1000 pictures or so with
1) No patch and no tag attached during import
2) No patch and    tag attached during import
3) Patch    and no tag attached during import
4) Patch    and    tag attached during import

I do not think we need to update any old database, since the patch automatically checks for this "Last Import" tag, and if it does not exist creates it. Since the user might delete the tag I think this is quite ok solution.

==> When you remove the last "Last Import" tag, before you set the new tag, you have another for loop and commit on each photo. Wouldn't it be possible to do something like "DELETE FROM photo_tags WHERE tag_id = {0}" This should go much faster.

==> I also saw that you have two lookups for "GetTagByName("Last Import");"
Is this needed? Could it not be solved with only one lookup?


In short
--------
1) Delete old Last Import tag (delete from photo_tags...)
   (or delete + update if keeping, say, 10 Import sessions)
2) Create Last Import tag
3) Loop through imported photos
  a) If user specified a tag, add it
  b) Add the Last Import tag
  c) commit

--------------------
I would really like to have the last 5-10 imports though. Not to sure how this should be implemented.
Perhaps a tag hierarchy like this
Last Imports
  --> 0
  --> 1
  --> 2
  --> 9
But this would demand the following SQL statements (or something like this showing 5 last imports here)
DELETE FROM photo_tags WHERE tag_id = <9>
UPDATE photo_tags tag_id=5 WHERE tag_id = 4
UPDATE photo_tags tag_id=4 WHERE tag_id = 3
UPDATE photo_tags tag_id=3 WHERE tag_id = 2
UPDATE photo_tags tag_id=2 WHERE tag_id = 1
UPDATE photo_tags tag_id=1 WHERE tag_id = 0
(automatically create the new tag if it did not exist already)
How much longer time would this take?
Comment 9 Bengt Thuree 2006-05-19 09:01:18 UTC
Created attachment 65811 [details] [review]
Add 6 last imports as tags

Modified Gunnars patch a bit.
Main differences
* Tracks 6 last imports
* Only one commit per photo

Any comments or feedbacks?
Comment 10 Bengt Thuree 2006-05-19 09:10:44 UTC
Before I forget :)
Remaining things todo (maybee) on patch in comment #9.

1) Ensure no new tag get the same name as "Latest Imports" or __# where # = 0-9
2) Ensure you can not set the Tag during import to any of the Latest Imports associated tags (Latest Imports, __#)

Comment 11 Bengt Thuree 2006-05-19 16:31:08 UTC
The patch do not update the photos when it is shifting __0 -> __1, which means that the photo might have __0 as a tag, but the database could have __1.
This is not that good.
Only way to fix this is
1) Always update the photos ---> Takes way to long time.
2) Skip updating the photos for these tags --> Complicated and not nice
3) Don't use tags for Last Import, but store directly in database. 

3) Is nicest, but it complicates everything when you want to use these 'Last Import tags', since you would have normal tags, and these extra "Last Import" things. Would be nice to be able to use the tag query branch together with these extra things...
Comment 12 Bengt Thuree 2006-05-21 13:07:38 UTC
See also bug #342482
Comment 13 Bengt Thuree 2006-05-31 06:37:25 UTC
Created attachment 66509 [details]
Menu screenshot

I have added some menus for this patch.
1) Last Import Roll
2) Last Import Rolls...
3) Clear Last Import Rolls
Comment 14 Bengt Thuree 2006-05-31 06:38:55 UTC
Created attachment 66510 [details]
Last Roll Filter gui

Added a small GUI so user can select which import roll to work with.
Yes, the gui could be improved... :) Someone willing to have a go?
Comment 15 Bengt Thuree 2006-05-31 06:40:12 UTC
Created attachment 66511 [details]
Last Roll Query 

And added some indication to the filter gui.
Comment 16 Bengt Thuree 2006-05-31 06:51:48 UTC
Created attachment 66513 [details] [review]
Patch

This patch will keep track of the latest 20 imports.
There is a small gui to select which roll(s) to filter upon, as well as a fast speed to latest imported photos. 
A number of files had to be touched to implement this.

I have also created a small updater that only adds one extra table (imports_data) (So to do a rollback, either have a backup, or delete this imports_data, and the contents in imports)

Would appreciate some review and comments so we can get this one ready for commit. :)
Comment 17 Stephane Delcroix 2006-05-31 09:52:28 UTC
Review and comments.

Some pros:
-Apply cleanly and compile
-Update the database without problems.
-Nice UI

And some issues:
-try this sequence:
  -Choose 'Find>Last import roll'
  -Choose 'Find>Last Import rolls...' ans select an import
  -everything is shown ! (probably a ui problem)
-also had this issue (and trying to reproduce it):
  -import some images
  -import some others
  -the first image of the second import is shown as part of the first import

Comment 18 Stephane Delcroix 2006-05-31 12:16:32 UTC
Crash when importing from an empty directory

way to reproduce:
- f-spot CVS + attach 66513
- create an empty directory
- import, Select Folder, select your empty folder
- 'change your mind', click on 'Cancel', 'Select Folder' or 'Include subdirectories'
- and crash

Scanning /home/test/empty
System.NullReferenceException: Object reference not set to an instance of an object
in <0x00010> DbStore:RemoveFromCache (.DbItem item)
in <0x00017> ImportStore:Remove (.DbItem item)
in <0x00011> ImportStore:DeleteCurrent ()
in <0x00297> FileImportBackend:Cancel ()
in <0x00036> ImportCommand:Cancel ()
...
Comment 19 Bengt Thuree 2006-05-31 12:41:47 UTC
Thanks for the feedback.
The crash have been located and fixed on my system. 
Trying to find the other small issue...
Comment 20 Stephane Delcroix 2006-05-31 13:25:22 UTC
more infos to reproduce 2nd sequence of comment #17

- add some photos
- remove 1 from the set
- add another set of photos
- select the first import

the problem is that f-spot is recycling photoid's.

here's the db after the sequence:
sqlite> select * from photos;
1|1149084501|/home/sde/Desktop|HPIM0241-800.jpg||1
2|1149084501|/home/sde/Desktop|HPIM0242-800.jpg||1
3|1149084501|/home/sde/Desktop|HPIM0243-800.jpg||1
4|1149084501|/home/sde/Desktop|HPIM0244-800.jpg||1
5|1149084032|/home/test/folder1|P1010072.JPG||1
6|1149084032|/home/test/folder1|P1010073.JPG||1
7|1149084032|/home/test/folder1|P1010074.JPG||1
8|1149084032|/home/test/folder1|P1010075.JPG||1
9|1149084032|/home/test/folder1|P1010076.JPG||1
10|1149084032|/home/test/folder1|P1010077.JPG||1
11|1149084032|/home/test/folder1|P1010078.JPG||1
12|1149084032|/home/test/folder1|P1010079.JPG||1

and that's the query for retrieving the first roll:
sqlite> select photo_id from imports_data where import_id in (1);
1
2
3
4
5

But it's not happening everytime (recycling id's).
Comment 21 Stephane Delcroix 2006-05-31 13:51:07 UTC
and how to fix the comment #20 ...

in the schema for the 'photos' table, change "id INTEGER PRIMARY KEY NOT NULL" by "id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL" to ensure that "The id chosen for the new row is one larger than the largest id that has ever before existed in that same table." (from http://www.sqlite.org/autoinc.html)

my $.02€, for what it worth...
Comment 22 Stephane Delcroix 2006-05-31 13:59:37 UTC
Can keep my $.02, it worth nothing.
There is a more clean way to do it in PhotoStore.cs in 'public void Remove (Photo []items)'. just copy/paste for removing from imports_data
Comment 23 Bengt Thuree 2006-05-31 15:24:14 UTC
Created attachment 66542 [details] [review]
Updated patch

Fixed the small crash, and ensure you can go from Last Import Roll to gui. Added deleting photo_id from imports_data when photo is deleted as well.
Comment 24 tjb 2006-05-31 18:52:21 UTC
I would prefer to keep all imports instead of just the last 20. It might even simplify the code.
Comment 25 Bengt Thuree 2006-05-31 23:59:48 UTC
I created another small patch (bug #342482) that kept all the import times.
The main difference between this and that patch, is that this one do not modify photos table. Otherwise they are identical in that they store an import id, which can be converted to the importtime you imported this batch (time per import batch, not per photo). This one have the gui added to it.

It is very simply to remove the purging of old data from this patch. But should the user be allowed to select whichever import session of the 1000 you have done in the gui? Not quite sure how to do this in an easy way.

==> Is there a strong request to be able to show when a photo was added to F-Spot?
In my case the key date/time is when I took the photo, and to be able to work with, say, the last 20 import sessions to set the tags properly.

==> If we want to keep the import time for all photos, should the photos db have a new column, or as in this patch a separate table.
Comment 26 Stephane Delcroix 2006-06-07 07:50:17 UTC
... no longer apply cleanly against CVS...
Comment 27 Bengt Thuree 2006-06-07 09:53:49 UTC
Created attachment 66895 [details] [review]
updated patch against CVS

... thanks...
now it does... I hope :)
Comment 28 Stephane Delcroix 2006-06-07 10:41:54 UTC
Created attachment 66898 [details] [review]
updated updated patch against CVS

the previous patch misses the LatRollDialog.cs file
Comment 29 Bengt Thuree 2006-06-07 11:03:59 UTC
Created attachment 66902 [details] [review]
Double updated patch against cvs

Crap! Forgot to add the newly added LastRollDialog.cs file :(
Comment 30 Stephane Delcroix 2006-06-07 12:57:49 UTC
patch review: patch 66902 for bug 321770

some pros:
- apply against CVS, compile cleanly
- definitively useful !
- working as expected

some minor cons:
- when you remove all pictures from the import, the 'Last Import Rolls' UI say '07 Jun (15:27) - Imported  photos'. Two way to fix it: zero IS a valid number, or simply remove imports when they do not contains pictures any more
- rename the 'Clear Last Import Rolls'. I was wondering what's the usage for this. My expectations were that it will clean the db from all previous imports, but no. After looking at the code, it clear the query. and it's well done. but the name is not intuitive...

some answers to your questions:
==> Is there a strong request to be able to show when a photo was added to
F-Spot?
No, finding the 20 last imports is enough for me
 
==> If we want to keep the import time for all photos, should the photos db
have a new column, or as in this patch a separate table.
If we keep all the import sessions, no need to add field, we can find it out! But as I already said, it's enough to know about the last 20 ones

some other things related to the patch:
- the 'Last import Rolls' need a bit more love from an UI designer (why not a dropdown list with multiple selects ?)
- for the last imported pictures, it could be helpful to display the import time, or the import roll in the infobox on the left...
Comment 31 Bengt Thuree 2006-06-07 14:49:39 UTC
I am updating (enabling/disabling) the menus in the UpdateMenus function. 
I guess it is not called when you remove photos...
Comment 32 Bengt Thuree 2006-06-07 15:02:46 UTC
sorry, disregard what I just wrote.

In PhotoStore, I delete the photo from imports_data, but do not delete the import session if there are no more photos attached to this import.
This is done during the next import session (purge).

Hmm... Will probably have to do this during delete as well. 
Comment 33 Bengt Thuree 2006-06-08 00:53:33 UTC
Created attachment 66944 [details] [review]
Tripple updated patch against cvs

Fixed the menu a bit
1) Renamed from "Clear Last Import Rolls" to "Clear Last Import Filter"
2) Checking for number of import sessions in imports_data instead of in imports, since when you delete a photo, it is only deleted in imports_data.
3) Purging old empty (due to deleted photos) sessions when setting a filter

Regarding displaying the import time... If we should have this, then perhaps we should keep the import times for all photos, not only the last 20 import sessions?

GUI could use some face lift for sure :) not sure a dropdown list is the best though, then again, I am definitely not a gui wizard :)
Comment 34 Bengt Thuree 2006-06-08 01:14:47 UTC
Changing title
Comment 35 Bengt Thuree 2006-07-22 09:35:58 UTC
Created attachment 69376 [details] [review]
Patch to apply after applying 342137

This is an updated patch that applies cleanly after patch for bug #342137 (id #69375) has been applied.

Created after 'popular' demand...

Nothing has been changed though.
Comment 36 Bengt Thuree 2006-08-03 14:07:17 UTC
Created attachment 70137 [details] [review]
Bengst Patch against CVS

Using gconf to store the number of rolls to keep in history.
New gui (thanks to Stephane)

This patch applies cleanly to CVS Head.
Comment 37 Stephane Delcroix 2006-08-04 16:02:10 UTC
patch review, patch #70137 for bug #321770

* the apply against CVS and compile
* as I early said, it's damn useful !
* it works as expected

in ImportStore.cs:
* the ImportComparerByDate and the method Compare can be made static

in MainWindow.cs:
* why not replacing
    clear_last_import_filter.Sensitive = (last_import_rolls.Sensitive);
by
    clear_last_import_filter.Sensitive = (last_import_roll.Sensitive);
* why not having both "Last Roll" and "Last Rolls" as CheckMenuItem ?
* the logic inside the Handle...()'ers seems a bit weird, I need a second deeper look to that part

in Preferences.cs:
* there's no default in GetDefault(). I know that the Get is called with a default argument, so I don't know if ti's required

in LastRolls.cs
* rename the on_... methods to Handle...
* We_Dont_Use_That_Case, ButThatOne in Update_... Populate_... Get_...
* I don't understand why having a LastRoll class, then an internal Set class with an Execute method. Probably need to rename all this, having only a LastRollsDialog : GladeDialog class
* why not using the constructor of the base class to build the UI, but call CreateDialog in Execute() ?

But nice patch afterall ;)
Comment 38 Bengt Thuree 2006-08-07 14:07:27 UTC
Thanks for the review, as always it is very much appreciated.
I have implemented some of the above comments, explained some, and am happy to implement some others, if there is a push for it.

ImportStore.cs : Static - Done

MainWindow.cs : Clear Last Import Filter should only be active if you have more than 1 import roll. If you only have one import roll, then the tick box on Last Import Roll filter should be "unticked" to clear the filter.

I would really prefere not to have "Last Rolls" as a CheckMenuItem, since that introduces a much more complicated was to handle it. Everytime you automatically change the tick mark on it (clear filter, last import roll) , the callback function will be called. Also, next time you click on it, the tick box dissapeares, when you instead wanted to just modify the filter roll gui.

The logic in the handler, is a bit weird (I think also), due to the CheckMenuItem's automatic callback's when we by code modify the status.

Preference default : Not required, since the default is specified in the call.

LastRolls.cs
* Rename on_ to Handle_ - Done

Not done the rest...

UpperCase: The coding guideline do not state clear what the function name should have, but the internal variables should have "_"
I based this patch on an existing module in F-Spot, which had the internal Set class. Agree with you though, perhaps should modify this.
Comment 39 Alexandre Prokoudine 2006-09-22 07:56:00 UTC
Even after removing Changelog part od the latest diff it doesn't cleanly aply to CVS:

avp@traveller:~/soft/build/graphics/f-spot/f-spot$ patch -p0 < LastImportRolls.diff 
patching file src/CameraFileSelectionDialog.cs
patching file src/FileImportBackend.cs
Hunk #1 FAILED at 13.
Hunk #2 succeeded at 242 with fuzz 1 (offset 11 lines).
Hunk #3 FAILED at 287.
Hunk #4 succeeded at 307 with fuzz 2 (offset 14 lines).
Hunk #5 succeeded at 337 (offset 14 lines).
2 out of 5 hunks FAILED -- saving rejects to file src/FileImportBackend.cs.rej
patching file src/ImportCommand.cs
Hunk #3 succeeded at 735 (offset 1 line).
Hunk #4 succeeded at 752 (offset 1 line).
patching file src/ImportStore.cs
patching file src/MainWindow.cs
Hunk #3 succeeded at 1060 (offset 3 lines).
Hunk #4 succeeded at 1068 (offset 3 lines).
Hunk #5 succeeded at 1269 (offset 3 lines).
Hunk #6 succeeded at 2328 (offset 14 lines).
Hunk #7 succeeded at 2571 (offset 14 lines).
patching file src/Makefile.am
patching file src/PhotoQuery.cs
patching file src/PhotoStore.cs
Hunk #1 succeeded at 1048 (offset 6 lines).
Hunk #2 succeeded at 1164 (offset 6 lines).
Hunk #3 succeeded at 1254 (offset 6 lines).
Hunk #4 succeeded at 1312 (offset 6 lines).
Hunk #5 succeeded at 1379 (offset 6 lines).
Hunk #6 succeeded at 1404 (offset 6 lines).
patching file src/Preferences.cs
Hunk #1 succeeded at 81 with fuzz 2 (offset 10 lines).
patching file src/QueryDisplay.cs
patching file src/Updater.cs
patching file src/f-spot.glade
Hunk #1 FAILED at 7272.
Hunk #2 FAILED at 7625.
Hunk #3 FAILED at 7635.
Hunk #4 succeeded at 8888 (offset 1179 lines).
Hunk #5 succeeded at 14591 (offset 1427 lines).
3 out of 5 hunks FAILED -- saving rejects to file src/f-spot.glade.rej
patching file src/LastRollDialog.cs
Comment 40 Bengt Thuree 2006-09-25 09:45:44 UTC
The vote was to keep all import dates permanently, so I will try and merge this patch with bug #342482

Next update to this bug will be a patch which stores import session id permanently per photo.
Comment 41 Thomas Van Machelen 2006-09-25 11:40:44 UTC
Do you have an idea of how you are going to treat photos that got imported before this patch gets applied?  Are you going to treat them as one big import, or as "not imported"?

I once mentioned on the mailing list that "querying" the imports should maybe be done through the time/directory widget by adding an extra "ImportAdapter" (like there now is a TimeAdaptor and a Directoryadapter) instead of through a separate dialog.

If i find some time, i might have a go at it when you update your patch.
Comment 42 Bengt Thuree 2006-09-25 11:53:20 UTC
One big import or not imported. They will all have the same import session id.

Not the best, but what can I do...

ok, I guess one way, would be to do it week by week or something like that.
But that might be a lot of code for this in the Update function. Would it be worth it?
Comment 43 Bengt Thuree 2006-09-26 14:20:29 UTC
Created attachment 73433 [details] [review]
Patch with permanent import session in photo

Ok, here is the first draft (merge rather).
I will continue to check and tweak it over the next week or so, but comments are always welcome.

This patch WILL modify your photos, and add one extra field (import_sessions).
DO NOT TEST THIS ONE ON YOUR REAL DATA YET!
I will probably change a bit, and possible the datastructure, so only use this one on a test installation.
Comment 44 Bengt Thuree 2006-10-01 13:11:36 UTC
Created attachment 73748 [details] [review]
Patch with permanent roll id in photo

I have renamed import_id to roll_id, and renamed table imports to rolls
(The ImportStore.cs is still called ImportStore.cs though)

Also, updated so it compiles cleanly against cvs.

Recommend to test this one with a test user or --basedir  option.

Please test, and comment :)
Comment 45 Bengt Thuree 2006-10-15 05:52:57 UTC
Any comments on this one?
The database structure is ok or?

Adding one extra field to Photos, as well as starting using the Imports table?
Comment 46 Bengt Thuree 2006-10-21 22:15:49 UTC
*** Bug 363941 has been marked as a duplicate of this bug. ***
Comment 47 Bengt Thuree 2007-04-09 14:40:04 UTC
Any comments thoughts on this one?

Comment 48 Stephane Delcroix 2007-04-13 20:22:18 UTC
Bengt,

this patch/functionality will more than probably be part of the next release. The lastest patch need some work, at least to make it patch the svn... let me know if you have some time to do it,... or I'll do

s
Comment 49 Bengt Thuree 2007-04-13 23:42:28 UTC
Ok

I can see if can find some time to do it, but for sure if it pops up on your top list feel free to do it. 

Looking forward to get this one in :)
Will be perfect when to use when I start to use f-spot regularly.
Comment 50 Bengt Thuree 2007-04-19 14:49:45 UTC
Created attachment 86636 [details] [review]
Patch with permanent roll id in photo

I have updated this patch (took some time, and a lot of changes has been made to the f-spot source code).

Please feel free to test this one as much as possible.
I have only done minor testing so far...
Comment 51 Bengt Thuree 2007-04-19 14:55:42 UTC
Created attachment 86637 [details] [review]
Patch with permanent roll id in photo

This patch is identical to the last one, apart from the diff of the glade file is a bit more sane.
Comment 52 Bengt Thuree 2007-04-19 15:36:43 UTC
Created attachment 86642 [details] [review]
Patch with permanent roll id in photo

Ok, this patch also includes the code to update the tag filter status bar.
(The original file had been renamed and refactored sometime before)
Comment 53 Alexandre Prokoudine 2007-04-22 16:25:05 UTC
Latest patch works great for me. Here is a couple of notes:

1. In the Search bar "Import roll" should have a hover hint that will display chosen criterias like "After April 22, 2007 (30)".

2. Double-clicking "Import roll" in the Search bar should open up the "Find -> Last Import rolls..." dialog.

3. A horizontal line just above OK/Cancel in the Last Import Rolls dialog doesn't seem to be HIG-friendly ;-)

The first and second notes imply rethinking approach to search filters. Let me explain this.

Last Import Roll (LIR) is a filter of the same hierarchy level that Set Date Range (SDR) is. But LIR is displayed in the search bar and SDR isn't. Thus, for the sake of consistency I suggest showing *all* search filters in the Search bar. A particular GUI for displaying them as groups of criterias is a job for UI engineers, however ;-)
Comment 54 Alexandre Prokoudine 2007-04-22 16:37:16 UTC
Additionally, it would be great beeing able to define several Last Import Roll criterias, like

a) between Roll1 and Roll2 AND/OR after Roll3;
b) between Roll1 and Roll2 AND/OR before Roll3 ('before' isn't present as of now);

The dialog would look like Banshee's one for smart playlists.
Comment 55 Stephane Delcroix 2007-04-23 08:09:19 UTC
hi alexandre,

I don't get your point... the AND operator, in bot a) and b) is, IMHO, useless.
The 'OR' thing has a possible marginal use but will probably add confusion.

I'd say let it as-is for now, we'll see if it need to be extended after inclusion in the trunk.

Don't worry, the code as it is now is already able to handle the additional cases...
Comment 56 Alexandre Prokoudine 2007-04-30 17:23:05 UTC
Another note on last version of the patch.

If I choose "after X roll" and then import new shots, the filtered view is not updated with new shots (though logically it should).
Comment 57 Stephane Delcroix 2007-05-23 17:36:48 UTC
Created attachment 88688 [details] [review]
reworked patch
Comment 58 Stephane Delcroix 2007-05-24 20:03:55 UTC
fixed in r3110