Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Export/Import deck options not working when using default options group #3023

Open
dae opened this issue Feb 24, 2024 · 14 comments · May be fixed by #3145
Open

[Bug] Export/Import deck options not working when using default options group #3023

dae opened this issue Feb 24, 2024 · 14 comments · May be fixed by #3145

Comments

@dae
Copy link
Member

dae commented Feb 24, 2024

@dae The issue appears when exporting the Default deck (The skip_default=true case in gather_decks())

Originally reported on https://forums.ankiweb.net/t/bug-export-import-deck-options-not-working/41428/3

If a deck is set to the default preset (dcid=1), options will be taken from the default preset in the importing collection, as we don't want shared decks to alter the defaults the user has configured. But this is confusing when "include deck configs" is enabled. Perhaps we should return an error if that option is enabled and the default config is encountered? Something like "Deck presets can not be included when your deck is using the default deck preset"?

@abdnh
Copy link
Contributor

abdnh commented Feb 28, 2024

Perhaps we should return an error if that option is enabled and the default config is encountered? Something like "Deck presets can not be included when your deck is using the default deck preset"?

Ideally we should clone the preset instead? Not sure how complex is that.

@dae
Copy link
Member Author

dae commented Mar 1, 2024

I think that might be problematic in the scenario where the user is repeatedly sharing a deck with others - we don't want to be creating a new deck preset each time we export, as they'd build up in the importing user's collection. And the type of people who want to export deck presets tend to be teachers sharing with students, or students sharing with friends, so repeated imports aren't too uncommon I suspect. Maybe it's simplest just to tell the user that they can't use the default deck preset if they want to share their presets? Perhaps we should also make the default preset easier to see, such as appending " (Default)" to the name in the dropdown?

@abdnh
Copy link
Contributor

abdnh commented Mar 3, 2024

Makes sense.

Perhaps we should also make the default preset easier to see, such as appending " (Default)" to the name in the dropdown?

Good idea.

@RumovZ
Copy link
Collaborator

RumovZ commented Mar 24, 2024

How about replacing the id on export with a fixed value like the collection's creation timestamp? Optionally, the replacement could be reversed on import (if exporter equals importer).

@dae
Copy link
Member Author

dae commented Mar 24, 2024

That'll break if the exporter imports their cards into a new collection, then exports them again, as the timestamp would be different. If we require the user to select a non-default preset, it's not going to break in such cases, and it lets us avoid extra complexity in the code, which is appealing. Hopefully changing the preset is not much to ask of users who are sharing?

@RumovZ
Copy link
Collaborator

RumovZ commented Mar 24, 2024

That'll break if the exporter imports their cards into a new collection, then exports them again, as the timestamp would be different.

Not sure I follow. Let's say we have an collection X created at t_X. On export, the deck config id 1 would be replaced with t_X. It could then be imported into and exported from collection Y, with the id remaining t_X. Only when importing into X again, the id could be interpreted as 1 – or just treated like any other id ≠ 1 and imported as a new deck config.

But I totally understand you preferring the explicit and less complex way. Just wanted to put all options on the table.

@dae
Copy link
Member Author

dae commented Mar 25, 2024

What I was thinking of is X's owner moves their cards into a new profile (thus new timestamp), and tries to share again. The importer would end up with a different deck config to the previous time, unless I'm missing something.

Options are always good :-) But I think simple is probably best in this case.

@RumovZ
Copy link
Collaborator

RumovZ commented Mar 25, 2024

If they move the cards via apkg, the id would be t_X (not 1) in the new profile, and any exports from there would not depend on the collection's creation timestamp.
If they move via colpkg, I think the creation timestamp is copied as well. So exports would depend on the creation timestamp, but it would be the same as in the old collection.

Sorry for flogging a dead horse here. 😄

@dae
Copy link
Member Author

dae commented Mar 26, 2024

I was assuming that when "include scheduling" is enabled in the export to the other profile, the deck config would remain unchanged, and be 1 in the new profile as well. I assumed this t_X transform would only apply when scheduling=false and deck_config=true. Were you thinking differently? Or perhaps I've missed something else? :-)

@RumovZ
Copy link
Collaborator

RumovZ commented Mar 26, 2024

I had only considered with_deck_configs. Why do you think the behaviour should depend on with_scheduling?

@dae
Copy link
Member Author

dae commented Mar 26, 2024

The original use case for include_scheduling was for users moving their own cards between profiles, so I tend to think of that option as "export for myself", though this applies less since deck_configs was split into a separate option. When exporting for oneself, keeping things the same as in the original collection is probably best, so the user doesn't need to switch their decks based from the new t_X preset back to the default preset if they want the exact setup as before. But that's not what the option is called, so this may be more me thinking about the original intent than what we're actually doing/presenting to the user ^_^;

@RumovZ
Copy link
Collaborator

RumovZ commented Apr 7, 2024

When exporting for oneself, keeping things the same as in the original collection is probably best

This case isn't supported if exporting the default config is prevented entirely by showing an error, either.
But I get the point: With an ad-hoc id it seems like you can export a perfect copy of a deck, but it doesn't completeley work; the deck config would be the same, but it wouldn't be the default config anymore.

However, thinking more about the error solution it seems overly restrictive. If I want to export multiple decks and only really care about non-default configs, I would still need to assign different configs to all decks with the default one first.

What about migrating the export screen to web and adding a tooltip about the special default config handling?

@dae
Copy link
Member Author

dae commented Apr 8, 2024

If I want to export multiple decks and only really care about non-default configs, I would still need to assign different configs to all decks with the default one first.

I'm trying to imagine a scenario where a deck sharer would want to pass some deck configs along, but export other (sub)decks with the default preset. Is it really unreasonable to expect the user to limit themselves to non-default presets when they use the 'include presets' option?

What about migrating the export screen to web and adding a tooltip about the special default config handling?

We'll want to migrate it at one point anyway, so no objections from me :-)

@RumovZ
Copy link
Collaborator

RumovZ commented Apr 8, 2024

I'm trying to imagine a scenario where a deck sharer would want to pass some deck configs along, but export other (sub)decks with the default preset.

For one thing, I was thinking about the export-for-myself use case. Then I might just as well want to export all decks at once. At the moment, having to synchronize the default preset some other way is not ideal, but manageable. I think it would be more annoying if you couldn't export your deck tree as-is at all.

We'll want to migrate it at one point anyway, so no objections from me :-)

I'll go ahead with it then!

RumovZ added a commit to RumovZ/anki that referenced this issue Apr 15, 2024
@RumovZ RumovZ linked a pull request Apr 15, 2024 that will close this issue
RumovZ added a commit to RumovZ/anki that referenced this issue Apr 28, 2024
RumovZ added a commit to RumovZ/anki that referenced this issue May 2, 2024
RumovZ added a commit to RumovZ/anki that referenced this issue May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants