SQLite DB with SQLAlchemy II

[yedpodtrzitko]

This takes some parts which has been done in #190 and keep working on it further

The transfer from JSON backend to SQL backend isnt as simple as replacing json.load() with db.select() so there is a lot of more refactoring to do before this will be production ready. The current JSON implementation suffers with memleaks (see #208) and is slightly over-engineered at some places, so that's yet another reason for a refactoring.

The reason why even open the PR at this time is to keep track of what's done, and to avoid potential duplication of work in case someone else would feel like working on the DB implementation. In such case, feel free to open PR against this branch.

Unfortunately I dont keep updating this branch with anything what's landing in the main branch, so by that time this will be any close to being done, the main branch might divert very far.

What works

• [x] opening library
• [x] displaying items
• [x] pagination
• [x] removing entry fields
• [x] adding entry fields
• [x] creating tags
• [x] items selection
• [x] searching via tag name

What does not work

• [x] everything else

[CyanVoxel]

This looks like a great barebones(ish) replacement to the JSON system so far, I think this is the best bet to get a SQL in the door without worrying about new schemas.

Taking a look at your comments, I'm keeping track of the features that still need to be implemented. So far I can tell there's at least:

• Select All functionality
• Various Macro functionality (was unofficial in the first place so not a priority)
• Ability to purge items from the NavigationState/FilterState
• Probably everything else that hasn't been touched?

From my own precursory testing, when opening an existing library with 200 items (191 visible with extension list filter), it comes back with supposedly 20 invisible results and tells me that most if not all the items are "not in library yet", such as with the following log snippet:

2024-07-24 23:49:35 [info     ] item not in library yet        path=WindowsPath('3D/d20 scene.mtl')
2024-07-24 23:49:35 [info     ] item not in library yet        path=WindowsPath('3D/d20 scene.obj')

Refreshing the library for new files also does not change anything here. Could be a case of the Windows paths acting funky somewhere, although that's just a surface-level guess.

I would also recommend basing this against the Alpha-v9.4 branch whenever it feels right, that way the changes there don't come as a sudden surprise. I don't think there's much going on there that will get in the way of this, but new changes are new changes nonetheless. I don't expect this to be ready by 9.4, but if it were then I would give this precedent over any new feature PRs. I feel that this would likely be included in 9.5 along with the new search syntax additions (now that will need some work resolving).

Please let me know what I can do to help speed this along or help smooth the transition in any way as well. Thank you so much for working on this!

[yedpodtrzitko]

@CyanVoxel I originally planned to polish this a bit more before getting it merged, but looking how the project got a second breath now, and a lot of new PRs are getting created, it would be better to get merged as soon as possible, so any new PRs would be based against the changes here. I'm not THAT much invested in this to be backporting every single PR into this branch. Working on this functionality makes me busy enough already, and not every single PR can be easily merged, if at all.

For example this PR #329 from the Alpha-9.4 branch for sorting tags - the sorting happens manually, whereas with DB you can just offload the sorting to the SQL query. Merging this PR would mean... I cant really merge it, because the underlying parts are now totally different.

Same goes with other library related PRs like #284 or #322 - the proposed changes are not possible to get merged, as they basically operate on a dictionary structure (ie. the loaded json) instead of the database.

Same applies with non-library PRs, for example #235 - it says "logging refactoring", but it's actually needlessly touching way more things, which will create a lot of git conflicts when merging that. And the list keeps going...

So even though not everything here is 100% finished, having it merged could speed up getting things done, as it wouldnt rely just on me, but other people could fix some of the issues and/or add required functionality to the database backend.

Regarding your previous comment - most of the problems you mentioned are already fixed, as a lot of changes landed here since then. I have also added some actually meaningful tests, and the tests are now testing the QT application as well, so that could help quite a lot to not introduce any regressions, and have well defined behaviour.

[CyanVoxel]

@CyanVoxel I originally planned to polish this a bit more before getting it merged, but looking how the project got a second breath now, and a lot of new PRs are getting created, it would be better to get merged as soon as possible, so any new PRs would be based against the changes here. I'm not THAT much invested in this to be backporting every single PR into this branch. Working on this functionality makes me busy enough already, and not every single PR can be easily merged, if at all.

For example this PR #329 from the Alpha-9.4 branch for sorting tags - the sorting happens manually, whereas with DB you can just offload the sorting to the SQL query. Merging this PR would mean... I cant really merge it, because the underlying parts are now totally different.

Same goes with other library related PRs like #284 or #322 - the proposed changes are not possible to get merged, as they basically operate on a dictionary structure (ie. the loaded json) instead of the database.

Same applies with non-library PRs, for example #235 - it says "logging refactoring", but it's actually needlessly touching way more things, which will create a lot of git conflicts when merging that. And the list keeps going...

So even though not everything here is 100% finished, having it merged could speed up getting things done, as it wouldnt rely just on me, but other people could fix some of the issues and/or add required functionality to the database backend.

Regarding your previous comment - most of the problems you mentioned are already fixed, as a lot of changes landed here since then. I have also added some actually meaningful tests, and the tests are now testing the QT application as well, so that could help quite a lot to not introduce any regressions, and have well defined behaviour.

First off, thank you so much for working so hard on this. This is insanely valuable to the project and it'll be a game changer to finally be working off of SQL!

This PR is my #1 priority for this project as far as I'm concerned. I'll take a look and see how things are operating now and get this merged into main as long as nothing critical in your migration list is broken. In the meantime, I'll probably do some sort of preview release for 9.4 based off of the current state of the thumbnails branch to get those features out in some form, and then divert my resources to helping get this polished off and have the 9.4 features backported to work with this over time.

[yedpodtrzitko]

@seakrueger thanks for the review, appreciate it.

[CyanVoxel]

I've been giving this a spin and taking notes, thank you again so much for working on this! I won't pester with non-critical stuff for this PR. Just a few questions and observations of relevant things to address before deciding to pull:

1. I noticed that new Tag IDs are starting after the internal ones instead of at 1000. Those IDs were previously reserved for required built-in tags, with "Favorite" and "Archived" being the only current ones. That way the program would have room to keep track of tags that need special treatment in some way via a consistently known ID. I wanted to get your take on that, whether we should continue with ID reservation or devise a more apt solution for the future, such as a dedicated table for required built-in tags. This is also likely going to be an issue with exportable/importable tags in the future, however fine-tuning these solutions is far out of the scope of what we're doing here. But there will need to be a decision on the fate of those reserved IDs before users are given the opportunity to populate them. TL;DR: I don't think it would hurt to reserve those IDs again now and contemplate alternative solutions later.
2. I see that every entry now has a "Meta Tags" field by default. Was this due to a requirement by the database (see 5.), or was this done in an effort to quicken the tagging process by having a premade field? If it's the later, I say let's double down and offer "Title", "Meta Tags", and "Content Tag" fields at least. A future PR can make this user-configurable.
3. Paths are currently being stored in the DB as OS-specific, however due to some nonsense this can lead to the issue described in #298. Part of this fix was going to be always storing the paths as POSIX inside the DB going forward. The rest of that fix involved converting the paths in existing libraries, but that can be saved as part of the JSON to SQL conversion process.
4. It doesn't seem like subtags are implemented yet, correct?
5. Lastly I wanted to note that the Favorite + Archived badges act very out of sync with the entries when in multiselection and when manually adding/removing the Favorite and Archived tags or fields containing them from entries, even after refreshing the pages. The strangest behavior I've been able to replicate is deleting the premade Meta Tags field, clicking the favorite button, and not getting the Favorite tag to show until adding the Meta Tags field back in. Anyway, I don't mind if badges aren't working for this PR, I just wanted to make sure that there weren't any deeper DB issues related to it.

Thank you so much again for all this!

[yedpodtrzitko]

1. I noticed that new Tag IDs are starting after the internal ones instead of at 1000. 

will add an item into the checklist, thanks for bringing this to my attention

2. I see that every entry now has a "Meta Tags" field by default. Was this due to a requirement by the database (see 5.), or was this done in an effort to quicken the tagging process by having a premade field? If it's the later, I say let's double down and offer "Title", "Meta Tags", and "Content Tag" fields at least. A future PR can make this user-configurable.

I think it was just because when I wanted to test anything related to tags, I had to add a new field each time, which gets old pretty fast. I can either remove that, or add all the other fields you mentioned.

3. Paths are currently being stored in the DB as OS-specific

added into the checklist

4. It doesn't seem like subtags are implemented yet, correct?

db structure is prepared for both parent tags and subtags, but I havent tried to actually create any, so I cant confirm nor deny this.

5. Lastly I wanted to note that the Favorite + Archived badges act very out of sync with the entries when in multiselection and when manually adding/removing the Favorite and Archived tags or fields containing them from entries, even after refreshing the pages. The strangest behavior I've been able to replicate is deleting the premade Meta Tags field, clicking the favorite button, and not getting the Favorite tag to show until adding the Meta Tags field back in. Anyway, I don't mind if badges aren't working for this PR, I just wanted to make sure that there weren't any deeper DB issues related to it.

added into checklist

[CyanVoxel]

[...]

will add an item into the checklist, thanks for bringing this to my attention

[...]

I think it was just because when I wanted to test anything related to tags, I had to add a new field each time, which gets old pretty fast. I can either remove that, or add all the other fields you mentioned.

[...]

added into the checklist

[...]

db structure is prepared for both parent tags and subtags, but I havent tried to actually create any, so I cant confirm nor deny this.

[...]

added into checklist

Thank you for the response! For the addition of default fields, I say let's go ahead with that. Probably more convenient than having none, and again this can be made user-configurable in a future PR. Everything else sounds good! 👍

[yedpodtrzitko]

@eivl thanks for the review!

[yedpodtrzitko]

@CyanVoxel apart from the last three items in the checklist, everything else should be done. I presume the tag-related items wont need any big changes, so it can be eventually done in main branch as well, so I'd say it's ready to get it merged to unblock merging other things, and more people can get their hands on it to test everything.

[CyanVoxel]

@CyanVoxel apart from the last three items in the checklist, everything else should be done. I presume the tag-related items wont need any big changes, so it can be eventually done in main branch as well, so I'd say it's ready to get it merged to unblock merging other things, and more people can get their hands on it to test everything.

Awesome!! Thank you so much for this again!

[CyanVoxel]

I'm having some new difficulties with the imports since the last time I pulled this. I've made sure to reinstall the dependencies via requirements.txt and requirements-dev.txt. Have any of the dependencies or versions changed in the past couple weeks? Or may I be missing something obvious?

Windows 10, VSCode:

Running pip install -r requirements.txt: Requirement already satisfied: greenlet!=0.4.17 in f:\github\tagstudio\.venv\lib\site-packages (from SQLAlchemy==2.0.34->-r requirements.txt (line 14)) (3.0.3)

Running pip install greenlet: Requirement already satisfied: greenlet in f:\github\tagstudio\.venv\lib\site-packages (3.0.3)

Running pip list:

Package                   Version
------------------------- --------
greenlet                  None

Along with the rest of the modules, with a few others having version "None" as well.

[Computerdores]

the dependencies are not an issue for me (I ran my pip install commands after switching branch though), however closing the library is broken for me:

Traceback (most recent call last):
  File "C:\Users\Jann\Downloads\TagStudio\TagStudio\src\qt\ts_qt.py", line 326, in <lambda>
    close_library_action.triggered.connect(lambda: self.close_library())
                                                   ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Jann\Downloads\TagStudio\TagStudio\src\qt\ts_qt.py", line 579, in close_library
    self.lib.clear_internal_vars()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Library' object has no attribute 'clear_internal_vars'

[yedpodtrzitko]

I'm having some new difficulties with the imports since the last time I pulled this. I've made sure to reinstall the dependencies via requirements.txt and requirements-dev.txt. Have any of the dependencies or versions changed in the past couple weeks? Or may I be missing something obvious?

sqlalchemy version was increased from 2.0.32 to 2.0.34, which shouldnt make difference, but try installing that older version to see if that would help? pip install sqlalchemy==2.0.32

You can also try uninstall and reinstall greenlet:

pip uninstall greenlet
pip install -U greenlet

I dont have greenlet installed in my virtualenv at all, so this is quite surprising error to see.

[yedpodtrzitko]

the dependencies are not an issue for me (I ran my pip install commands after switching branch though), however closing the library is broken for me:

Traceback (most recent call last):
  File "C:\Users\Jann\Downloads\TagStudio\TagStudio\src\qt\ts_qt.py", line 326, in <lambda>
    close_library_action.triggered.connect(lambda: self.close_library())
                                                   ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Jann\Downloads\TagStudio\TagStudio\src\qt\ts_qt.py", line 579, in close_library
    self.lib.clear_internal_vars()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'Library' object has no attribute 'clear_internal_vars'

fixed, thanks.

[CyanVoxel]

sqlalchemy version was increased from 2.0.32 to 2.0.34, which shouldnt make difference, but try installing that older version to see if that would help? pip install sqlalchemy==2.0.32

You can also try uninstall and reinstall greenlet:

pip uninstall greenlet
pip install -U greenlet

I dont have greenlet installed in my virtualenv at all, so this is quite surprising error to see.

Recreating the venv seems to have fixed it, and using either sqlalchemy 2.0.32 and 2.0.34 seems to work fine. Greenlet now lists the version as 3.0.3 in pip list 👍

[CyanVoxel]

So far I've come across a few errors or situations where the data acts strangely. I'm not sure how many of these you're already aware of, but I'll share them here regardless:

Error upon loading a SQLite library populated with tags applied to entries:

Show Logs

2024-09-06 20:05:37 [info     ] update_widgets                 selected=[]
2024-09-06 20:05:37 [info     ] opening library                connection_string=sqlite:///F:\Example SQL\.TagStudio\ts_library.sqlite
2024-09-06 20:05:37 [info     ] creating db tables
2024-09-06 20:05:38 [info     ] update_widgets                 selected=[]
2024-09-06 20:05:38 [info     ] searching library              filter=FilterState(page_index=0, page_size=500, search_mode=, tag=None, tag_id=None, id=None, path=None, name=None, query=None) query_full=SELECT entries.id, entries.path 
FROM entries
WHERE lower(entries.path) NOT LIKE lower('%..json,.xmp,.aae')
LIMIT 500 OFFSET 0
2024-09-06 20:05:38 [info     ] items to render                count=232
2024-09-06 20:05:38 [info     ] toggle_item_tag                entry_id=4 field_key=TAGS_META tag_id=1 toggle_value=True
2024-09-06 20:05:38 [error    ] Can't attach instance ; another instance with key (, (1,), None) is already present in this session.
Traceback (most recent call last):
  File "F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py", line 641, in add_field_tag
    field.tags = field.tags | {tag}
    ^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 537, in __set__
    self.impl.set(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1985, in set
    collections.bulk_replace(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 804, in bulk_replace
    appender(member, _sa_initiator=initiator)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1394, in add
    value = __set(self, value, _sa_initiator, NO_KEY)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1096, in __set
    item = executor.fire_append_event(item, _sa_initiator, key=key)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 688, in fire_append_event
    return self.attr.fire_append_event(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1752, in fire_append_event
    value = fn(state, value, initiator or self._append_token, key=key)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\unitofwork.py", line 70, in append
    sess._save_or_update_state(item_state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 3501, in _save_or_update_state
    self._save_or_update_impl(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4202, in _save_or_update_impl
    self._update_impl(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4191, in _update_impl
    self.identity_map.add(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\identity.py", line 195, in add
    raise sa_exc.InvalidRequestError(
sqlalchemy.exc.InvalidRequestError: Can't attach instance ; another instance with key (, (1,), None) is already present in this session.
2024-09-06 20:05:38 [info     ] update_widgets                 selected=[]
2024-09-06 20:05:38 [info     ] searching library              filter=FilterState(page_index=0, page_size=500, search_mode=, tag=None, tag_id=4, id=4, path=None, name=None, query=None) query_full=SELECT entries.id, entries.path 

Error upon adding a tag to an entry, in certain circumstances? I'm aware that subtags are not yet implemented, but I'm not sure if that's the direct cause of this error. I created a tag "Cat", then a tag "Kitten" with "Cat" as a subtag (didn't expect it to have any lasting effect), then applied "Cat" to an entry under content tags, then attempted to add "Kitten" as well, but failed. Creating a new tag and applying it to the same field worked as intended. Applying tag "Kitten" to "Meta Tags" also worked as intended.

Show Logs

2024-09-06 20:18:10 [info     ] add_tag_callback               selected=[4] tag_id=1001
2024-09-06 20:18:10 [error    ] Can't attach instance ; another instance with key (, (1000,), None) is already present in this session.
Traceback (most recent call last):
  File "F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py", line 641, in add_field_tag
    field.tags = field.tags | {tag}
    ^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 537, in __set__
    self.impl.set(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1985, in set
    collections.bulk_replace(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 804, in bulk_replace
    appender(member, _sa_initiator=initiator)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1394, in add
    value = __set(self, value, _sa_initiator, NO_KEY)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1096, in __set
    item = executor.fire_append_event(item, _sa_initiator, key=key)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 688, in fire_append_event
    return self.attr.fire_append_event(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1752, in fire_append_event
    value = fn(state, value, initiator or self._append_token, key=key)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\unitofwork.py", line 70, in append
    sess._save_or_update_state(item_state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 3507, in _save_or_update_state
    self._save_or_update_impl(st_)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4202, in _save_or_update_impl
    self._update_impl(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4191, in _update_impl
    self.identity_map.add(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\identity.py", line 195, in add
    raise sa_exc.InvalidRequestError(
sqlalchemy.exc.InvalidRequestError: Can't attach instance ; another instance with key (, (1000,), None) is already present in this session.

Error after creating tag "Cat", then searching for "cat". Results show as intended, but a traceback remains:

Show Logs

2024-09-06 20:12:36 [info     ] searching library              filter=FilterState(page_index=0, page_size=500, search_mode=, tag='cat', tag_id=None, id=None, path=None, name=None, query='cat') query_full=SELECT entries.id, entries.path 
FROM entries JOIN tag_box_fields ON entries.id = tag_box_fields.entry_id JOIN tag_fields AS tag_fields_1 ON tag_box_fields.id = tag_fields_1.field_id JOIN tags ON tags.id = tag_fields_1.tag_id
WHERE (lower(tags.name) LIKE lower('cat') OR lower(tags.shorthand) LIKE lower('cat')) AND lower(entries.path) NOT LIKE lower('%..json,.xmp,.aae')
 LIMIT 500 OFFSET 0
2024-09-06 20:12:36 [info     ] items to render                count=8
2024-09-06 20:12:36 [info     ] toggle_item_tag                entry_id=4 field_key=TAGS_META tag_id=1 toggle_value=True
2024-09-06 20:12:36 [error    ] Can't attach instance ; another instance with key (, (1,), None) is already present in this session.
Traceback (most recent call last):
  File "F:\GitHub\TagStudio/tagstudio\src\core\library\alchemy\library.py", line 641, in add_field_tag
    field.tags = field.tags | {tag}
    ^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 537, in __set__
    self.impl.set(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1985, in set
    collections.bulk_replace(
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 804, in bulk_replace
    appender(member, _sa_initiator=initiator)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1394, in add
    value = __set(self, value, _sa_initiator, NO_KEY)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 1096, in __set
    item = executor.fire_append_event(item, _sa_initiator, key=key)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\collections.py", line 688, in fire_append_event
    return self.attr.fire_append_event(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\attributes.py", line 1752, in fire_append_event
    value = fn(state, value, initiator or self._append_token, key=key)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\unitofwork.py", line 70, in append
    sess._save_or_update_state(item_state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 3501, in _save_or_update_state
    self._save_or_update_impl(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4202, in _save_or_update_impl
    self._update_impl(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\session.py", line 4191, in _update_impl
    self.identity_map.add(state)
  File "f:\GitHub\TagStudio\.venv\Lib\site-packages\sqlalchemy\orm\identity.py", line 195, in add
    raise sa_exc.InvalidRequestError(
sqlalchemy.exc.InvalidRequestError: Can't attach instance ; another instance with key (, (1,), None) is already present in this session.
2024-09-06 20:12:36 [info     ] update_widgets                 selected=[2]

When switching between previews of entries with or without Title text, sometimes the text is not updated between switching entries, and also appears heavily shifted to the right. I can't pinpoint the conditions to cause this at the moment.

Additionally:

• When editing tags, the color is automatically shifted forward by one index - I remember reading a comment in the code about this, just wanted to bring it up again.
• I don't seem to be able to add new fields to entries
• Clicking on tags to search for them via ID no longer works (I wouldn't worry too much about this, since after this PR would be a great time to get rid of the ID search syntax in favor of a better search bar UI anyways)

[CyanVoxel]

I'm also still fine with pulling this in a state like this, I just wanted to make sure there wasn't anything critically wrong before doing so

[yedpodtrzitko]

I will go through the notes you posted, and I will also do one more futureproofing change.

[yedpodtrzitko]

Error upon loading a SQLite library populated with tags applied to entries:

should be fixed

I created a tag "Cat", then a tag "Kitten" with "Cat" as a subtag (didn't expect it to have any lasting effect), then applied "Cat" to an entry under content tags, then attempted to add "Kitten" as well, but failed.

fixed

Error after creating tag "Cat", then searching for "cat". Results show as intended, but a traceback remains:

should be also fixed

When switching between previews of entries with or without Title text, sometimes the text is not updated between switching entries, and also appears heavily shifted to the right. I can't pinpoint the conditions to cause this at the moment.

🤔 I dont think this could be caused by the db backend, and I cant even replicate that, but I'll keep an eye on it

Additionally:

* When editing tags, the color is automatically shifted forward by one index - I remember reading a comment in the code about this, just wanted to bring it up again.

fixed

* I don't seem to be able to add new fields to entries

fixed

* Clicking on tags to search for them via ID no longer works (I wouldn't worry too much about this, since after this PR would be a great time to get rid of the ID search syntax in favor of a better search bar UI anyways)

fixed

[CyanVoxel]

I'll check out these changes soon and get back to you 👍

In the meantime, I noticed that the apprun.yaml workflow was removed. Was that intentional?

[yedpodtrzitko]

In the meantime, I noticed that the apprun.yaml workflow was removed. Was that intentional?

Yes, that's on purpose. It happened that there was some error when running the app a few times, but apprun job was still green. I replaced it with in tests/qt. Each of these is testing a different component of the app, so the overall coverage is much better. It's also more granular so when a test for a specific component fails, you know where to start looking.

[CyanVoxel]

Thank you again SO MUCH for all of your hard work on this!! 🎉

[Tishj]

To answer a TODO that was left in the code by this PR:

        if result == 3:  # TODO - what is this magic number?
            callback()

The answer is here, it's an enum: https://doc.qt.io/qt-6/qmessagebox.html#ButtonRole-enum

The exec() slot returns the StandardButtons value of the button that was clicked.

Actually, reading that page more carefully, it should be this instead:

diff --git a/tagstudio/src/qt/widgets/preview/field_containers.py b/tagstudio/src/qt/widgets/preview/field_containers.py
index 5c1cde8..a96d69a 100644
--- a/tagstudio/src/qt/widgets/preview/field_containers.py
+++ b/tagstudio/src/qt/widgets/preview/field_containers.py
@@ -499,11 +499,11 @@ class FieldContainers(QWidget):
         cancel_button = remove_mb.addButton(
             Translations["generic.cancel_alt"], QMessageBox.ButtonRole.DestructiveRole
         )
-        remove_mb.addButton("&Remove", QMessageBox.ButtonRole.RejectRole)
+        confirm = remove_mb.addButton("&Remove", QMessageBox.ButtonRole.RejectRole)
         remove_mb.setDefaultButton(cancel_button)
         remove_mb.setEscapeButton(cancel_button)
-        result = remove_mb.exec_()
-        if result == 3:  # TODO - what is this magic number?
+        remove_mb.exec()
+        if remove_mb.clickedButton() == confirm:
             callback()

     def emit_badge_signals(self, tag_ids: list[int] | set[int], emit_on_absent: bool = True):