GNOME Bugzilla – Bug 321770
Keeping track of last 20 import sessions
Last modified: 2007-05-24 20:03:55 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.
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.
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
*** Bug 328357 has been marked as a duplicate of this bug. ***
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
what about an "imported:" or "import session started:" field, or an "import sessions" table with the images linking to it?
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.
I will have a look at this patch in the next day or so.
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?
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?
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, __#)
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...
See also bug #342482
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
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?
Created attachment 66511 [details] Last Roll Query And added some indication to the filter gui.
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. :)
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
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 () ...
Thanks for the feedback. The crash have been located and fixed on my system. Trying to find the other small issue...
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).
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...
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
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.
I would prefer to keep all imports instead of just the last 20. It might even simplify the code.
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.
... no longer apply cleanly against CVS...
Created attachment 66895 [details] [review] updated patch against CVS ... thanks... now it does... I hope :)
Created attachment 66898 [details] [review] updated updated patch against CVS the previous patch misses the LatRollDialog.cs file
Created attachment 66902 [details] [review] Double updated patch against cvs Crap! Forgot to add the newly added LastRollDialog.cs file :(
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...
I am updating (enabling/disabling) the menus in the UpdateMenus function. I guess it is not called when you remove photos...
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.
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 :)
Changing title
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.
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.
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 ;)
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.
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
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.
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.
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?
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.
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 :)
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?
*** Bug 363941 has been marked as a duplicate of this bug. ***
Any comments thoughts on this one?
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
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.
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...
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.
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)
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 ;-)
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.
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...
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).
Created attachment 88688 [details] [review] reworked patch
fixed in r3110