Personal Fields + Import Config Window

[ohare93]

Hello again! 😁 I come to you today with a change which is very important to me: the ability to be able to import a CrowdAnki json file with some fields set as "Personal" which then keep the old field data before the import. Sort of like a merge 🤔 this would fix my issue #62

However this is just a small proof of concept, only 1 file is changed! 😅 while brewing over how to do this change I came to the conclusion that this is a very fiddly issue which could be done in many different ways. Therefore I'd rather come to you early in the process and lay out the options and my thoughts. Perhaps there's an easier way I was not seeing 😄 😏 Regardless, I'll save myself some hassle down the line by doing it this way now, I feel.

---

Proof of concept

It is rather simple, as each field in an Anki flashcard is serialized to a string in CrowdAnki json, even if it is blank. I simply added a check for if a field value is false, and if so I overwrite it with the current value of that field before updating the card as a whole 👍 This is a rather simplistic approach, and perhaps may be considered hack-y, but it gets the job done.

So the current implementation is done on a per note basis and currently only works on importing (with a field set to false). This has many downsides, and I would prefer the ability to set an entire field as "Personal", as that would allow for:

• Export notes without "Personal" fields
• Default values on first import of a card

The first one can simply be an export option to select, but for a deck with many many types of cards it can get annoying. Not to mention if a hosted deck has some "Personal" fields, it should be simple and seamless to simply export those as "Personal" too, as the default.

So how to set a whole field as "Personal" is the tough question 🤔 🤕 Here are some ideas:

1. Check for a special string start/end of the field name, such as "PERSONAL_Picture" or "NotesPERSONAL".
   Good for hosted decks, bad for user choice and clarity.
2. Get each user to set the "Personal" fields in the Config settings.
   Good for user choice, bad that they have to set it up for each deck they download.
3. A mixture of both: a flag of "Personal: true" on the deckmodel, which the importer checks and registers this notemodel as having that field as "Personal" in each user's config 🤔

For exporting there can always be a tickbox "Override Personal Fields", or some such, which allows you to set more or less fields as "Personal" than what is in your config.

Anyways, that's my brain dump over 😅 Let me know what you think, whenever you have time 👍

[Stvad]

As a general comment - look into using rebase instead of merges for git to keep the history cleaner

[Stvad]

I think it'd be a good idea to outline the use-cases here, which would help with design decisions (i.e. whether the personal field metadata should be part of the deck or part of user configuration)

Things I've heard so far:

• Personal notes field
• Copyrighted material field (This one is confusing for me, as presumably you'd want the exclusion here to work not on the field level but on individual notes level, as you still want to include the things with an appropriate license?)

Not entirely sure if possible, but may be neat:

• Local override field - example would be a deck that has shortcuts for some program, and you may want to customize shortcuts. Then you'd add your custom shortcuts in the override field. Note display logic would need to have a way to conditionally display things on cards though.

[Stvad]

Given that fields are defined in the NoteModel - it can be a better vessel for the field type information (so my default intuition would be closer to the option 1 you've outlined on #62). + Allowing user choice on how to deal with personal fields (i.e. have import/export personal fields config/checkbox on corresponding dialogs)

There is a question of transferring that information between Anki instances this discussion may be relevant

[Stvad]

Regarding the poc - It works 🎉. I'm not sure if doing things on per note basis scales though ;) Also I'd probably prefer to have the explicit configuration over convention in this case

[ohare93]

As a general comment - look into using rebase instead of merges for git to keep the history cleaner

Yea sorry, will do 😭

[ohare93]

I think it'd be a good idea to outline the use-cases here, which would help with design decisions

Definitely! 👍

Things I've heard so far:

Personal notes field
Copyrighted material field (This one is confusing for me, as presumably you'd want the exclusion here to work not on the field level but on individual notes level, as you still want to include the things with an appropriate license?)

I think it would be better for it to be on the field level, to just have a {{Picture}} field which does not sync to the users, but that they can set themselves. I am not against the concept of a doing it on a per note basis, as that may be useful in some other way, but I'd like it to be settable in the notemodels 👍

[ohare93]

Since I have a POC version of this I've been busy with other stuff that will actually use this. But just to be clear, I am open to doing it whatever way 🤷‍♂ your preference would be for it to be on the notemodel, and I am fine with implementing that.

However my main goal is to get importing personal fields functional. It would also be nice to be able to export, and to have saved export settings, and to offer to automagically set those export settings when you import a deck with personal fields, etc, etc. However all of that comes with extra issues and complexity, and is not needed for me right now 😅

So I suppose I am asking: if I implement support of a Personal Field just in the importing of a CrowdAnki json file (by adding an entry to the corresponding Notemodel definition), is that good enough as an MVP of this feature? I am happy to extend it to Exporting, then be smart and save user settings, etc, but not in the next few months, I expect 😅

[Stvad]

You can definitely work on things incrementally. Start by implementing importing on NoteModel level, and it can be marked as experimental feature initially. And then work towards full end-end support over time :)

[ohare93]

Work in progress, was delayed greatly due to my Anki development build being broken. Spent two days on it, but have just given up to actually get something useful done. Who even needs breakpoints? 😭

Anyways, I have created an Import window, and interrupted the flow of Import to display it before the actual importing begins. The current idea of the implementation is that it reads in the "_settings" key from the json (open to a different name! 😅) then displays that to the user for them to update in a proper UI window, then after the user accepts their import settings it does the import as normal, passing those new values to the importer to use in various places.

I also have a sense that the exporter will be the exact same in terms of settings, so I plan to just join them together into one import/export settings window. On export it reads in the settings from the existing config (such as sort method, etc) and allows the user to do a one time change before export. On import it does the same, but also reads in the settings from the json file itself and performs any overrides.

Thoughts behind this structure as a plan? 🤔

P.S: This import window is quite powerful as a concept, I plan to add a few extra goodies into there after Personal Fields 😉

[Stvad]

@ohare93 Welcome back! :) I haven't done it in a while, but I remember setting up the debugging being challenging =\

A few thoughts from your description:

• After the import is complete, where/how do we store the value of settings (as it's not imported to the anki db)?
• I don't want to see the dialog each time I do import/export. I think there should be a way to say "accept whatever is in settings by default" (a meta-setting?)

[ohare93]

• After the import is complete, where/how do we store the value of settings (as it's not imported to the anki db)?

The values are not intended to be saved, just passed down through the import/export logic to change the data in various ways. It essentially says "here are the import/export setting values stored in both your config and the import file itself, do you want to continue with these settings?"

• I don't want to see the dialog each time I do import/export. I think there should be a way to say "accept whatever is in settings by default" (a meta-setting?)

Well it's just one popup with nothing you have to do on it, so you can just click ok if you really don't care. Having that show up once on import is not a big deal in my view, and a "don't ask me about it again setting" kind of breaks the concept of "here are the values stored in the import file" 😥

Some of the settings I would like for deck managers to be able to set include:

• Personal Fields (already discussed 😉) presets, which the user can disable individually, or even add more before import
• Inform user of changes to notes or note structure (see Inclusion rules for political entities anki-geo/ultimate-geography#306 (comment)). Notes may have been deleted since their last update, or Note Models have changed. A place for a small explanatory message would go a long way.
  • Offer to tag all the imported notes, so the user can find the ones they may want to delete.
  • Tickbox for "Duplicate Note Model" in case they want to keep the old version, or want all their old notes to say on the old version. (Technically done now when there is a detected change, as the old note models aren't deleted, right? 🤔 having it as an option to do at any point would be useful, regardless)

And some options it would be good to give the user in this window on import/export:

• Import/Export Headers / Note Models / Notes / Media?
• Import/Export filter.

[ohare93]

Update: Working PR 👍

The new Import Window looks like this!

The general idea is that this data is prefilled with data from the import file (and the user's default settings in the config), then the user has the option to change each of them for this individual import. On pressing Okay the app continues the normal import flow, but a new variable import_config (open to a new name 😅) is passed down to each of the possible affected parts.

Personal Fields

The Personal Fields section of the window is autofilled with all the note models used in the import, showing all their fields. The fields are ticked by default if it finds a "personal_field": true inside the note_model field of the import.

                {
                    "font": "Liberation Sans",
                    "media": [],
                    "name": "Picture",
                    "ord": 2,
                    "personal_field": true,     <---------------------------
                    "rtl": false,
                    "size": 6,
                    "sticky": false
                },

Users can then deselect or select each of the fields and choose import. Each field that is selected as a Personal Field is added into a list of Tuples (note_model, field_name), and each card checks if it's own fields are in the list on save to collection (see note.py/handle_dictionary_update for the specifics).

Everything Else

I added a few extra features into this window as it was quite bare, and I wanted to actually settle on a good setup for passing an import_config with multiple values, rather than just one personal_fields object. Making these other values work was a piece of cake. I also hope these show the power of this import structure! 😁

Import Message

The import message is just a way for a deck manager to communicate with a deck user on import, to warn of potential changes, inform what they are, or whatnot. This can be set in the _settings value of a CrowdAnki import, but will function perfectly fine without it.

    "_settings": {
        "import_message": "This release has deleted cards X, Y, and Z. Please sort that out for yourself using blahblahblah.",
        "suggest_tag_imported_cards": true
    }

Tag Imported Cards

Tick this box and fill in a comma separated list of tags. Each card imported will receive that tag, as well as the ones on the cards themselves. The hope is that users will be able to more easily be able to separate cards imported in this run from others not affected, or perhaps deleted cards.

There is also a "suggest_tag_imported_cards" option that can be set in the _settings which came about as an idea from the discussion here: anki-geo/ultimate-geography#306 (comment) All this does is auto tick this box and add "Suggested by deck manager!" to the title.

Do Not Move Existing Cards

The config value I added a while ago, but now you can set it for each import. Defaults to the value in the ConfigSettings.

Deck Parts to Use

Not yet functioning! Apart from media, that works. The hope is that you can simply import the parts of the CrowdAnki import that you want, ignoring the rest. But this one may need some gooood testing and configuration 😅 just left it here to show it as a possibility, and am happy to disable the non working ones should we get to PR stage soon.

Anyways that's all for now, I am happy to have a working and usable version. Also what do people think about PersonalFields being inside the NoteModel? 🤔 I did that at the start, thinking I had to put it there, but it could just as easily go inside the new _settings value as a dictionary. Thoughts?

[Stvad]

some initial comments:

• On reflection I think it'd be preferable to separate the meta information into distinct section of the JSON file (or maybe even a separate file, it can be Yaml then :p) instead of mixing it in the field directly. So you can have something like

meta:
  note_models:
     model_uuid_1:
       personal_fields:
         - id1
         - id2

Added some comments on the XML for this. All headers could use emphasis.
Also the distinction between different note models should be more distinct in the list (headers emphasized, adding separators. Also should be clearer that different note models is what we're seeing

[Stvad]

should be separate function

[ohare93]

What should? Adding the fields into the QListWidget? 🤔

[ohare93]

Made all your little changes 👍 Lemme know what else you spot 👀

[ohare93]

What should? Adding the fields into the QListWidget? 🤔

[ohare93]

Review complete, and all requested changes made. Thanks to both of you 😁 🤯

What brought you here @agentydragon? 🙂 thanks for the review comments regardless, and glad to see a new face!

[ohare93]

Maintainer! Now that's a nice catch all that doesn't necessarily require it to be the owner of the repo. I swing my vote behind maintainer 😁

On Sat, 4 Jul 2020, at 8:31 PM, Vladyslav Sitalo wrote: ***@***.**** commented on this pull request. In crowd_anki/importer/import_dialog.py <#72 (comment)>: > + heading_ui = QListWidgetItem(model["name"])

+ heading_ui.setFlags(Qt.ItemIsEnabled) + self.form.list_personal_fields.addItem(heading_ui) + + for field in model["flds"]: + field_ui = QListWidgetItem(field["name"]) + field_ui.setCheckState(Qt.Checked if is_field_personal(field) else Qt.Unchecked) + field_ui.setFlags(Qt.ItemIsEnabled | Qt.ItemIsUserCheckable) + self.form.list_personal_fields.addItem(field_ui) + + def setup_misc(self): + self.form.import_message_textbox.setText(self.import_settings.import_message) + + if self.import_settings.suggest_tag_imported_cards: + self.form.cb_tag_cards.setCheckState(Qt.Checked) + self.form.cb_tag_cards.setText("Tag Cards (Suggested by Deck Manager!)")

…

eh, I still like the "Curator" best, owner has different connotations for me :p. how about "Maintainer" ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#72 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALHG5XR7VBVE56IVYOOOC3RZ5YQ7ANCNFSM4JO2CIXQ>.

[ohare93]

Merged and release?! 🤯 🥳 Fantastic, thank you for your many many reviews @Stvad 👏 we got there in the end 😁 🎉