JS function to setTags and getTags #17795

Haz3-jolt · 2025-01-11T13:35:04Z

Purpose / Description

Adds two JS API functions [set/replace]NoteTags and getTagsFromNote which act similar to PUT and GET from a REST API

Fixes

Fixes FR: JS API function to remove a specific tag #17716

Approach

Adds two methods to allow the caller to get Tags and update/set tags.

How Has This Been Tested?

Added two unit tests both verified and passing.

Checklist

Please, go through these checks before submitting the PR.

You have a descriptive commit message with a short title (first line, max 50 chars).
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

Haz3-jolt · 2025-01-11T13:37:13Z

@david-allison Do you think we should deprecate addTagToNote? Also [set/replace]NoteTags triggers lint failure

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt

mikehardy

Do you think we should deprecate addTagToNote

Yes - haven't done our deprecation strategy in a while, not sure exactly how. Is it documented anywhere? May not be, but we should be able to mark something as deprecated and have it emit a warning in console or similar

Haz3-jolt · 2025-01-11T16:06:25Z

Do you think we should deprecate addTagToNote

Yes - haven't done our deprecation strategy in a while, not sure exactly how. Is it documented anywhere? May not be, but we should be able to mark something as deprecated and have it emit a warning in console or similar

From some searching, the most I can find is the deprecated decorator on Kotlin side, I dunno how we would approach on js. Is there official documentation for JS api? if not we can just have a comment?

Haz3-jolt · 2025-01-11T16:09:38Z

Thanks for bringing this up. Here's my perspective...

Hey! Can you you elaborate further?

mikehardy · 2025-01-11T16:19:40Z

@Mizokuiam please stop. You are wasting everyone's time.

Haz3-jolt · 2025-01-11T16:22:20Z

@Mizokuiam please stop. You are wasting everyone's time.

Spammer? :(

mikehardy · 2025-01-11T16:33:15Z

@Haz3-jolt I assume so. Negative value contributions, banned at organization level by me now.

AnkiDroid/src/main/assets/scripts/js-api.js

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt

Haz3-jolt · 2025-01-11T18:41:33Z

@mikehardy How is this for a deprecation warning? Since people mainly interact on Js side

Subject: [PATCH] JS function to setTags and getTags
---
Index: AnkiDroid/src/main/assets/scripts/js-api.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/assets/scripts/js-api.js b/AnkiDroid/src/main/assets/scripts/js-api.js
--- a/AnkiDroid/src/main/assets/scripts/js-api.js	(revision 6d3d270e9067fbad4797f25b0f3060adc60e41cd)
+++ b/AnkiDroid/src/main/assets/scripts/js-api.js	(date 1736620228061)
@@ -121,6 +121,7 @@
 Object.keys(jsApiList).forEach(method => {
     if (method === "ankiAddTagToNote") {
         AnkiDroidJS.prototype[method] = async function (noteId, tag) {
+            console.warn("ankiAddTagToNote is deprecated. Use ankiSetNoteTags instead.");
             const endpoint = jsApiList[method];
             const data = JSON.stringify({ noteId, tag });
             return await this.handleRequest(endpoint, data);

mikehardy · 2025-01-11T18:50:20Z

@Haz3-jolt I believe that deprecation in javascript console is the correctly location for it

Haz3-jolt · 2025-01-11T18:54:03Z

@Haz3-jolt I believe that deprecation in javascript console is the correctly location for it

Amend the current commit or separate commit?

mikehardy · 2025-01-11T18:56:28Z

amend I think, the deprecation of the old is part of adding the new to me, it's all one piece

Haz3-jolt · 2025-01-11T18:57:17Z

amend I think, the deprecation of the old is part of adding the new to me, it's all one piece

Cool!

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt

david-allison · 2025-01-21T06:10:55Z

We should be using the Tags class, or at least a collection method to convert the tags to and from a list of strings

Otherwise we risk edge cases

Haz3-jolt · 2025-01-21T08:28:52Z

We should be using the Tags class, or at least a collection method to convert the tags to and from a list of strings

Otherwise we risk edge cases

"setNoteTags" -> {
    val jsonObject = JSONObject(apiParams)
    val noteId = jsonObject.getLong("noteId")
    val tags = jsonObject.getJSONArray("tags")
    withCol {
        val note = getNote(noteId).apply {
            this.tags = this@withCol.tags.split(tags.stringIterable().joinToString(" "))
        }
        updateNote(note)
    }
    convertToByteArray(apiContract, true)
}

Would this be better? It uses split from the tags class in libanki

Haz3-jolt · 2025-01-25T08:44:50Z

We should be using the Tags class, or at least a collection method to convert the tags to and from a list of strings
Otherwise we risk edge cases

"setNoteTags" -> {
    val jsonObject = JSONObject(apiParams)
    val noteId = jsonObject.getLong("noteId")
    val tags = jsonObject.getJSONArray("tags")
    withCol {
        val note = getNote(noteId).apply {
            this.tags = this@withCol.tags.split(tags.stringIterable().joinToString(" "))
        }
        updateNote(note)
    }
    convertToByteArray(apiContract, true)
}

Would this be better? It uses split from the tags class in libanki

@david-allison I still can't see how this would prevent any edge cases, any pointers?

david-allison · 2025-01-25T15:44:14Z

(not at my PC, DM me if this is ambiguous)

I believe we have another Tags class which should be extended to handle this case

I don't like the manual splitting of tags, we want to use the same logic each time (via a class/method), or a bug can lead to data corruption

Haz3-jolt · 2025-01-25T16:23:51Z

(not at my PC, DM me if this is ambiguous)

I believe we have another Tags class which should be extended to handle this case

I don't like the manual splitting of tags, we want to use the same logic each time (via a class/method), or a bug can lead to data corruption

So i can directly use the method from Utils.kt in api?
I'll have to change the object from internal to public, is that fine?

Haz3-jolt · 2025-01-25T16:50:32Z

Subject: [PATCH] rEDO
---
Index: api/src/main/java/com/ichi2/anki/api/Utils.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/api/src/main/java/com/ichi2/anki/api/Utils.kt b/api/src/main/java/com/ichi2/anki/api/Utils.kt
--- a/api/src/main/java/com/ichi2/anki/api/Utils.kt	(revision 633f2a37a0e95b34207494740f63a3321632d4aa)
+++ b/api/src/main/java/com/ichi2/anki/api/Utils.kt	(date 1737822495862)
@@ -24,7 +24,7 @@
 /**
  * Utilities class for the API
  */
-internal object Utils {
+public object Utils {
     // Regex pattern used in removing tags from text before checksum
     private val stylePattern = Pattern.compile("(?s)<style.*?>.*?</style>")
     private val scriptPattern = Pattern.compile("(?s)<script.*?>.*?</script>")
@@ -34,15 +34,15 @@
     private const val FIELD_SEPARATOR = '\u001f'.toString()
 
     // TODO: Add contract for null -> null and non-null to non-null when kotlin contracts become stable/out of experimental phase
-    fun joinFields(list: Array<String>?): String? = list?.joinToString(FIELD_SEPARATOR)
+    public fun joinFields(list: Array<String>?): String? = list?.joinToString(FIELD_SEPARATOR)
 
-    fun splitFields(fields: String): Array<String> =
+    public fun splitFields(fields: String): Array<String> =
         fields
             .split(FIELD_SEPARATOR.toRegex())
             .dropLastWhile { it.isEmpty() }
             .toTypedArray()
 
-    fun joinTags(tags: Set<String?>?): String {
+    public fun joinTags(tags: Set<String?>?): String {
         if (tags.isNullOrEmpty()) {
             return ""
         }
@@ -52,9 +52,9 @@
         return tags.joinToString(" ")
     }
 
-    fun splitTags(tags: String): Array<String> = tags.trim { it <= ' ' }.split("\\s+".toRegex()).toTypedArray()
+    public fun splitTags(tags: String): Array<String> = tags.trim { it <= ' ' }.split("\\s+".toRegex()).toTypedArray()
 
-    fun fieldChecksum(data: String): Long {
+    public fun fieldChecksum(data: String): Long {
         val strippedData = stripHTMLMedia(data)
         return try {
             val md = MessageDigest.getInstance("SHA1")
Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(revision 633f2a37a0e95b34207494740f63a3321632d4aa)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(date 1737822029568)
@@ -58,6 +58,7 @@
 import org.json.JSONException
 import org.json.JSONObject
 import timber.log.Timber
+import com.ichi2.anki.api.Utils
 
 typealias JvmBoolean = Boolean
 typealias JvmInt = Int
@@ -383,7 +384,7 @@
                 withCol {
                     val note =
                         getNote(noteId).apply {
-                            setTagsFromStr(this@withCol, tags.stringIterable().joinToString(" "))
+                            setTagsFromStr(this@withCol, Utils.joinTags(tags.stringIterable().toSet()))
                         }
                     updateNote(note)
                 }

@david-allison Here is a patch of changes if it's all good, ill push changes.

david-allison · 2025-01-25T19:26:08Z

Something like this

Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(revision 633f2a37a0e95b34207494740f63a3321632d4aa)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(date 1737833155918)
@@ -45,6 +45,7 @@
 import com.ichi2.libanki.Card
 import com.ichi2.libanki.Collection
 import com.ichi2.libanki.Decks
+import com.ichi2.libanki.Note
 import com.ichi2.libanki.SortOrder
 import com.ichi2.utils.NetworkUtils
 import com.ichi2.utils.stringIterable
@@ -377,13 +378,19 @@
             }
 
             "setNoteTags" -> {
+
                 val jsonObject = JSONObject(apiParams)
                 val noteId = jsonObject.getLong("noteId")
                 val tags = jsonObject.getJSONArray("tags")
                 withCol {
+                    fun Note.setTagsFromList(tagList: List<String>) {
+                        val tagsAsString = [email protected](tagList)
+                        setTagsFromStr(this@withCol, tagsAsString)
+                    }
+
                     val note =
                         getNote(noteId).apply {
-                            setTagsFromStr(this@withCol, tags.stringIterable().joinToString(" "))
+                            setTagsFromList(tags.stringIterable().toList())
                         }
                     updateNote(note)
                 }

Haz3-jolt · 2025-01-26T05:51:26Z

Something like this

Index: AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(revision 633f2a37a0e95b34207494740f63a3321632d4aa)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt	(date 1737833155918)
@@ -45,6 +45,7 @@
 import com.ichi2.libanki.Card
 import com.ichi2.libanki.Collection
 import com.ichi2.libanki.Decks
+import com.ichi2.libanki.Note
 import com.ichi2.libanki.SortOrder
 import com.ichi2.utils.NetworkUtils
 import com.ichi2.utils.stringIterable
@@ -377,13 +378,19 @@
             }
 
             "setNoteTags" -> {
+
                 val jsonObject = JSONObject(apiParams)
                 val noteId = jsonObject.getLong("noteId")
                 val tags = jsonObject.getJSONArray("tags")
                 withCol {
+                    fun Note.setTagsFromList(tagList: List<String>) {
+                        val tagsAsString = [email protected](tagList)
+                        setTagsFromStr(this@withCol, tagsAsString)
+                    }
+
                     val note =
                         getNote(noteId).apply {
-                            setTagsFromStr(this@withCol, tags.stringIterable().joinToString(" "))
+                            setTagsFromList(tags.stringIterable().toList())
                         }
                     updateNote(note)
                 }

Got it, thanks for the help!

Haz3-jolt · 2025-01-26T06:20:32Z

@david-allison did the final changes, and confirmed that all tests are passing.

david-allison

Testing for a tag with a space, or trailing whitespace would be useful

AnkiDroid/src/test/java/com/ichi2/anki/AnkiDroidJsAPITest.kt

david-allison

Should be the last one!

AnkiDroid/src/main/assets/scripts/js-api.js

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt

Haz3-jolt · 2025-01-31T10:30:39Z

@david-allison Any other changes? Ive double checked that both functions are working.

Haz3-jolt · 2025-02-03T16:49:45Z

@david-allison although I don't think I can do it rn, any idea on how a integration test for the setTags can be done? Perhaps setting a note with the Js function in the note, and using espresso to find the button or something?

Haz3-jolt · 2025-02-13T18:07:54Z

@david-allison should I re base and force push to check if any conflicts are there? its been a while since this PR has been open

Haz3-jolt · 2025-02-13T18:32:04Z

@david-allison should I re base and force push to check if any conflicts are there? its been a while since this PR has been open

Okay so looks like no conflicts

david-allison · 2025-02-14T09:32:31Z

The PR will display conflicts if there are any (and it'll have Has Conflicts )

If it's sat for a while, a rebase saves a little time on our end

david-allison

Cheers!

AnkiDroid/src/main/assets/scripts/js-api.js

mikehardy reviewed Jan 11, 2025

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt Outdated Show resolved Hide resolved

mikehardy reviewed Jan 11, 2025

View reviewed changes

This comment was marked as spam.

Sign in to view

Haz3-jolt force-pushed the setTags branch 2 times, most recently from 783f9fa to 36e8da6 Compare January 11, 2025 16:02

Haz3-jolt force-pushed the setTags branch from 36e8da6 to fbdca30 Compare January 11, 2025 16:08

david-allison reviewed Jan 11, 2025

View reviewed changes

AnkiDroid/src/main/assets/scripts/js-api.js Outdated Show resolved Hide resolved

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt Outdated Show resolved Hide resolved

Haz3-jolt force-pushed the setTags branch from fbdca30 to 6d3d270 Compare January 11, 2025 18:35

Haz3-jolt force-pushed the setTags branch from 6d3d270 to 3300a42 Compare January 11, 2025 18:57

david-allison added the Needs Review label Jan 11, 2025

david-allison approved these changes Jan 11, 2025

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt Outdated Show resolved Hide resolved

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Jan 11, 2025

Haz3-jolt requested a review from mikehardy January 12, 2025 12:31

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

Haz3-jolt force-pushed the setTags branch from 3300a42 to bc86eea Compare January 14, 2025 09:35

Haz3-jolt force-pushed the setTags branch from 633f2a3 to 8704aa9 Compare January 26, 2025 06:19

david-allison requested changes Jan 26, 2025

View reviewed changes

AnkiDroid/src/test/java/com/ichi2/anki/AnkiDroidJsAPITest.kt Outdated Show resolved Hide resolved

AnkiDroid/src/test/java/com/ichi2/anki/AnkiDroidJsAPITest.kt Outdated Show resolved Hide resolved

Haz3-jolt force-pushed the setTags branch 4 times, most recently from 4b7ec2b to 5b2f5bc Compare January 28, 2025 08:12

david-allison requested changes Jan 28, 2025

View reviewed changes

AnkiDroid/src/main/assets/scripts/js-api.js Show resolved Hide resolved

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidJsAPI.kt Outdated Show resolved Hide resolved

Haz3-jolt force-pushed the setTags branch from 5b2f5bc to a686527 Compare January 30, 2025 13:53

Haz3-jolt requested a review from david-allison February 1, 2025 12:51

Haz3-jolt force-pushed the setTags branch from a686527 to b152d33 Compare February 13, 2025 18:31

david-allison approved these changes Feb 14, 2025

View reviewed changes

AnkiDroid/src/main/assets/scripts/js-api.js Outdated Show resolved Hide resolved

AnkiDroid/src/main/assets/scripts/js-api.js Outdated Show resolved Hide resolved

david-allison added Needs Second Approval Has one approval, one more approval to merge and removed Needs Review labels Feb 14, 2025

Haz3-jolt added 2 commits February 14, 2025 13:54

JS function to setTags and getTags

f7166bc

Unit Test for getNoteTags and adding tags validation on Js

903c65f

Haz3-jolt force-pushed the setTags branch from b152d33 to 903c65f Compare February 14, 2025 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JS function to setTags and getTags #17795

JS function to setTags and getTags #17795

Haz3-jolt commented Jan 11, 2025 •

edited

Loading

Haz3-jolt commented Jan 11, 2025

mikehardy left a comment

This comment was marked as spam.

This comment was marked as spam.

Haz3-jolt commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

david-allison commented Jan 21, 2025

Haz3-jolt commented Jan 21, 2025

Haz3-jolt commented Jan 25, 2025

david-allison commented Jan 25, 2025

Haz3-jolt commented Jan 25, 2025 •

edited

Loading

Haz3-jolt commented Jan 25, 2025

david-allison commented Jan 25, 2025 •

edited

Loading

Haz3-jolt commented Jan 26, 2025

Haz3-jolt commented Jan 26, 2025

david-allison left a comment

david-allison left a comment

Haz3-jolt commented Jan 31, 2025

Haz3-jolt commented Feb 3, 2025

Haz3-jolt commented Feb 13, 2025

Haz3-jolt commented Feb 13, 2025

david-allison commented Feb 14, 2025 •

edited

Loading

david-allison left a comment

JS function to setTags and getTags #17795

Are you sure you want to change the base?

JS function to setTags and getTags #17795

Conversation

Haz3-jolt commented Jan 11, 2025 • edited Loading

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Checklist

Haz3-jolt commented Jan 11, 2025

mikehardy left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

Haz3-jolt commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

mikehardy commented Jan 11, 2025

Haz3-jolt commented Jan 11, 2025

This comment was marked as outdated.

This comment has been minimized.

This comment has been minimized.

david-allison commented Jan 21, 2025

Haz3-jolt commented Jan 21, 2025

Haz3-jolt commented Jan 25, 2025

david-allison commented Jan 25, 2025

Haz3-jolt commented Jan 25, 2025 • edited Loading

Haz3-jolt commented Jan 25, 2025

david-allison commented Jan 25, 2025 • edited Loading

Haz3-jolt commented Jan 26, 2025

Haz3-jolt commented Jan 26, 2025

david-allison left a comment

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

Haz3-jolt commented Jan 31, 2025

Haz3-jolt commented Feb 3, 2025

Haz3-jolt commented Feb 13, 2025

Haz3-jolt commented Feb 13, 2025

david-allison commented Feb 14, 2025 • edited Loading

david-allison left a comment

Choose a reason for hiding this comment

Haz3-jolt commented Jan 11, 2025 •

edited

Loading

Haz3-jolt commented Jan 25, 2025 •

edited

Loading

david-allison commented Jan 25, 2025 •

edited

Loading

david-allison commented Feb 14, 2025 •

edited

Loading