Export selected cards from CardBrowser

[TarekkMA]

Export individual cards from CardBrowser

Pull Request template

Purpose / Description

Following upstream, I believe it would be nice to be able to select card(s) in the deck browser and have an option to export them. It would be especially useful to share buggy card for debugging or to share a card type with an example without needing to share a whole deck or create it

Fixes

Fixes #8045

Approach

I have extracted all the needed logic for exporting cards and moved it to its own class ExportHelper. This class will delegate all actions from any activity that wish to implement exporting.

I also needed to change the ankilib to make it a colse as the pylib one

How Has This Been Tested?

• [x] Export normal deck
• [x] Export collection
• [x] Export individual cards from card browser

Learning (optional, can help others)

Describe the research stage

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

• [x] You have not changed whitespace unnecessarily (it makes diffs hard to read)
• [x] You have a descriptive commit message with a short title (first line, max 50 chars).
• [x] Your code follows the style of the project (e.g. never omit braces in if statements)
• [x] You have commented your code, particularly in hard-to-understand areas
• [x] You have performed a self-review of your own code
• [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)

[welcome[bot]]

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

[codecov[bot]]

Codecov Report

Merging #8159 (9517366) into master (205bbcb) will increase coverage by 0.05%. The diff coverage is 16.07%.

:exclamation: Current head 9517366 differs from pull request most recent head e0fc81c. Consider uploading reports for the commit e0fc81c to get more accurate results

@@             Coverage Diff              @@
##             master    #8159      +/-   ##
============================================
+ Coverage     36.97%   37.03%   +0.05%     
+ Complexity     3829     3793      -36     
============================================
  Files           358      356       -2     
  Lines         36201    35914     -287     
  Branches       4782     4755      -27     
============================================
- Hits          13386    13300      -86     
+ Misses        21298    21104     -194     
+ Partials       1517     1510       -7     

Impacted Files	Coverage Δ	Complexity Δ
...ain/java/com/ichi2/anki/dialogs/DialogHandler.java	13.04% <0.00%> (ø)	4.00 <0.00> (ø)
...a/com/ichi2/anki/dialogs/ExportCompleteDialog.java	0.00% <0.00%> (ø)	0.00 <0.00> (?)
...main/java/com/ichi2/anki/dialogs/ExportDialog.java	0.00% <0.00%> (ø)	0.00 <0.00> (ø)
.../src/main/java/com/ichi2/async/CollectionTask.java	36.09% <0.00%> (+0.83%)	15.00 <0.00> (-1.00)	:arrow_up:
...java/com/ichi2/anki/ActivityExportingDelegate.java	4.21% <4.21%> (ø)	1.00 <1.00> (?)
...roid/src/main/java/com/ichi2/anki/CardBrowser.java	47.39% <6.25%> (-0.14%)	131.00 <0.00> (ø)
...Droid/src/main/java/com/ichi2/anki/DeckPicker.java	20.35% <16.66%> (+0.77%)	56.00 <0.00> (ø)
...in/java/com/ichi2/libanki/AnkiPackageExporter.java	62.99% <82.35%> (+1.21%)	9.00 <0.00> (ø)
...kiDroid/src/main/java/com/ichi2/libanki/Utils.java	59.20% <100.00%> (+1.70%)	76.00 <2.00> (+4.00)
...oid/src/main/java/com/ichi2/utils/BundleUtils.java	85.71% <100.00%> (+10.71%)	6.00 <3.00> (+3.00)
... and 82 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 205bbcb...e0fc81c. Read the comment docs.

[TarekkMA]

Thank you about the wonderful feedback. I really appreciate you taking the time to write it.

I will try to implement some tests for the new ported code, will need to study other tests before writing my own. I'm really sorry about the miss I think android studio did some formatting before I pushed the code. I will try to fix that in its own commit.

I'm not yet familiar with git beyond basic operations, so I will read up on that and try to make more meaningful commits.

About GSoC, yes I contacted you on discord about this PR. I do have some level of experience in android but like you said the change wasn't big, I simply refactored and organized things up. But there is much more I can still learn from being a part of the internship. Being a contributor of a huge open source application like anki is a first for me. And I think anki have alot of things that can be done or learned that I'm not yet aware of. For example the TaskListenerWithContext with its implementation of WeakReference is something new, I just learned, and the use of custom web view to handle user content is also new to me.

Again I really appreciate your feedback, and I will try to follow your suggestions. Thank you

[TarekkMA]

I will not be able to work on this PR for the next few days, as I will be having exams this week. So I will try to work sparingly, or after I finish my exams.

[Arthur-Milchior]

Please take your time. There is no emergency, AnkiDroid spent 10 years without it, it can wait a month more. We are a tool to help people succeed academically, not make them fail :p. And everything is volunteering

[TarekkMA]

@Arthur-Milchior I have cleaned up the miss, and have divided commits into multiple logical ones, with appropriate messages.

Now there is no unrelated code formatting issues (lines, spaces). I have renamed ExportHelper to ActivityExportingDelegate.

Very eager to hear your thoughts about the new changes.

[Arthur-Milchior]

Please, if you did a change to some code and that there is no need to answer it, don't hesite to "resolve" the comment. There are a lot of it, and this will win time

[Arthur-Milchior]

Fun fact: I added to Anki the feature to select card in browser and export them. So this is partially my code that you're porting to java. It's a strange feeling to review it.

[TarekkMA]

Thanks a lot @Arthur-Milchior for the feedback.

I'm referring to it as "Individual cards" that the user have selected, maybe I'm using the expression wrong, if so should I change the commit messages?

I have tested to export it to another AnkiDriod(Old version) and to my PC, they both seem to work fine (will test it again after the new changes).

Will add nullable annotations and javadocs to the Exporter, and add missing formatting to this PR.

Should I remove setters exporter and refactor the ExportDialog.newInstance in the ~~same~~ new PR?

[Arthur-Milchior]

I'm more than fine with having a first PR about cleaning and a second one about the feature. You'll need to rebase current PR over the cleaning one, I'm sorry for that.

I'm not a native english speaker. "individual cards" seems a contradiction to me, but if you believe it's okay for multiple cards, I'm fine with it.

[TarekkMA]

No issues will push a new PR with that.

I'm also not a native speaker of English, so I would not know if this is 100% correct, but I feel like if this induces confusion we should rename it to something else? What do you suggest?

[Arthur-Milchior]

Export selected cards from Cardbrowser

[TarekkMA]

I have renamed the PR and the previous commit messages. Will work on the cleaning PR and submit it

[mikehardy]

This is going to be a very helpful feature, I agree with you @Arthur-Milchior ! Thanks @TarekkMA - and Arthur, when you feel this is good enough I will trust you for sure, you have worked so closely on reviewing it I'm not sure what I would add with limited time available

[TarekkMA]

@Arthur-Milchior I have rebased this PR over #8196. I think it's ready for a review

[TarekkMA]

@Arthur-Milchior can you have a look at this. This is a blocker to adding the CSV export.

The libanki porting is merged, what is left is this refactoring

[Arthur-Milchior]

There is quite a lot of conflict right now. Would you mind rebasing on master, resolving conflicts. This version clearly won't be merged as is and so I don't want to review it already

[TarekkMA]

@Arthur-Milchior done.

[TarekkMA]

Thanks for the feedback, I will split this into 2 PRs and will also rethink the delegate design since onActivityReuslt will be deprecated soon, and we should move the new API.

[Arthur-Milchior]

You force pushed. Do you want a review ? Or do i wait for the other pr first ?

[TarekkMA]

No was just rebasing on master and fixing conflicts.

I will split this into 2 PRs as requested.

There is a design chnage I have laid its details on discord. I will open a PR with such a change.

[github-actions[bot]]

Hello 👋, this PR has been opened for more than 2 months with no activity on it. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

[TarekkMA]

Keep Open

[TarekkMA]

This PR is hard to deconflict, I might try to recreate it into a fresh PR.

[dae]

The UI side of things will likely need updates for the Kotlin conversion. For the backend side of things, the Rust backend supports this use case (https://github.com/ankitects/anki/blob/f6390e9455365f696b0bd06f9be398e6698baf38/proto/anki/import_export.proto#L189), so utilizing it in AnkiDroid with the new schema would just require adding that case to the code (https://github.com/ankidroid/Anki-Android/blob/69da0c89e8795427156bb98afac66414f618a763/AnkiDroid/src/main/java/com/ichi2/libanki/BackendImportExport.kt#L106)

[Arthur-Milchior]

The UI side of things will likely need updates for the Kotlin conversion. For the backend side of things, the Rust backend supports this use case (https://github.com/ankitects/anki/blob/f6390e9455365f696b0bd06f9be398e6698baf38/proto/anki/import_export.proto#L189), so utilizing it in AnkiDroid with the new schema would just require adding that case to the code (

https://github.com/ankidroid/Anki-Android/blob/69da0c89e8795427156bb98afac66414f618a763/AnkiDroid/src/main/java/com/ichi2/libanki/BackendImportExport.kt#L106

)

Let's also ping @viciousAegis who is already working on the browser

[viciousAegis]

@dae I am unsure of how to implement the use cases for cardIds and noteIds mentioned in https://github.com/ankitects/anki/blob/f6390e9455365f696b0bd06f9be398e6698baf38/proto/anki/import_export.proto#L189 This is what I came up with, but it does not seem to be correct.

fun CollectionV16.exportAnkiPackage(
    outPath: String,
    withScheduling: Boolean,
    withMedia: Boolean,
    deckId: DeckId?,
    cardIds: List<CardId>?,
    noteIds: List<NoteId>?,
    legacy: Boolean = true,
) {
    val limit = exportLimit {
        if (deckId != null) {
            this.deckId = deckId
        } else if (cardIds != null) {
            this.cardIds.newBuilderForType().addAllCids(cardIds).build()
        } else if (noteIds != null) {
            this.noteIds.newBuilderForType().addAllNoteIds(noteIds).build()
        } else {
            this.wholeCollection = Empty.getDefaultInstance()
        }
    }

Could you guide me in the right direction regarding implementing cardIds and noteIds? Thanks :)

[dae]

Try this.cardIds.cidsList.addAll()

[viciousAegis]

Try this.cardIds.cidsList.addAll()

Returns java.lang.UnsupportedOperationException, I am assuming because cidsList is not mutable.

[dae]

this.cardIds = cardIds { cids.addAll() } ?

(this is import anki.cards.cardIds, not the cardIds param)