[JS Addons] Map package.json to AddonModel and check if is valid addon package or not by krmanik · Pull Request #10586 · ankidroid/Anki-Android

krmanik · 2022-03-19T10:00:43Z

Pull Request template

Purpose / Description

This is second PR in JS Addons series.
Before downloading addon tgz packages, it needs to check if the addon package is valid or not for AnkiDroid using package.json.
The validity checked using three fields addonTitle, ankiDroidJsApi and ankidroid-js-addon keyword which are not present in other npm package's package.json file. These fields will be used to distinguish it from other npm packages. It is required for addon developer to add these fields to package.json file otherwise the packages will not be downloaded and extracted.

Also it needs to check that name should be valid npm packages. The validatename method is implemented to check if name follow the naming convention of npm packages.

Fixes

Fixes #7959

Approach

A class with the fields that may be present in package.json file. Then mapper will be used to map package.json to AddonModel class.

val mapper = AnkiSerialization.objectMapper

updatePrefs method will be used to enable and disable addon, the prefs will stored as HashSet
isValidAnkiDroidAddon method have multiple checks like empty name or fields present or not in package.json and validateName for naming convention

How Has This Been Tested?

There are test class for each class.

Learning (optional, can help others)

https://github.com/npm/validate-npm-package-name
https://github.com/lassjs/is-valid-npm-name
https://stackoverflow.com/questions/50050436/use-npm-package-to-validate-a-package-name

Checklist

Please, go through these checks before submitting the PR.

You have not changed whitespace unnecessarily (it makes diffs hard to read)
You have a descriptive commit message with a short title (first line, max 50 chars).
Your code follows the style of the project (e.g. never omit braces in if statements)
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

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

mikehardy

Nice! I left some comments that are mostly about developer experience. The big question I have is about what is going in the SharedPrefs and that will affect how to get this finished I think

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/NpmUtils.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/NpmUtils.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

mikehardy

This LGTM with exception of the registry string as commented directly
This is great progress and I do think the logging messages will help the poor developer trying to figure something out when using the system :-)

AnkiDroid/src/main/res/values/02-strings.xml

mikehardy

LGTM!

mikehardy · 2022-03-24T14:11:43Z

Really happy to see these moving through, I know you basically have the whole system up and running already in your local builds @krmanik and I'm excited to have all the functionality go through and merge to main, even if it takes a little while for each chunk. We will get there!

krmanik · 2022-03-24T14:19:10Z

I am using this feature on local build. The split PR is faster than earlier single PR.

krmanik · 2022-03-29T01:00:02Z

Resolve the conflicts.

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

AnkiDroid/src/test/java/com/ichi2/anki/jsaddons/AddonModelTest.kt

mikehardy · 2022-04-13T15:12:03Z

This appears to be ready for a re-scan?

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

added timber log when validname checks return false

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

krmanik · 2022-04-15T17:25:00Z

In this commit 56a00b9, I have created AddonData class then mapped json then validate the data. After validation, created AddonModel and returned.
Is this correct implementation?

david-allison

Looks great! Two minor changes

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonData.kt

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonModel.kt

david-allison

One optional extension to make AddonModel fully immutable.

Happy to merge when CI passes

move test on json file to assets

mikehardy · 2022-04-15T19:54:43Z

Another JS Add Ons PR about to merge

mikehardy · 2022-04-15T20:07:39Z

@krmanik lint issue - I'm testing it locally:


* What went wrong:
Execution failed for task ':AnkiDroid:ktlintTestSourceSetCheck'.
> A failure occurred while executing org.jlleitschuh.gradle.ktlint.worker.ConsoleReportWorkAction
   > KtLint found code style violations. Please see the following reports:
     - /home/mike/work/AnkiDroid/Anki-Android-Upstream/AnkiDroid/build/reports/ktlint/ktlintTestSourceSetCheck/ktlintTestSourceSetCheck.txt

krmanik · 2022-04-15T20:13:41Z

Thanks, I am also running the lint.

mikehardy · 2022-04-15T20:17:29Z

passes all the other tests though, for me locally. Just some "needless blank lines" away from a merge :-)

/home/mike/work/AnkiDroid/Anki-Android-Upstream/AnkiDroid/src/test/java/com/ichi2/anki/jsaddons/AddonModelTest.kt:37:1 Needless blank line(s)

krmanik · 2022-04-15T20:19:49Z

I will push the changes.

change data to class and rename AddonModel to AddonData add values to error list change var to val to make it immuatable

david-allison · 2022-04-17T15:29:22Z

Do we have anywhere else to discuss this? It doesn't seem relevant to the PR.

krmanik · 2022-04-17T15:30:33Z

Do we have anywhere else to discuss this? It doesn't seem relevant to the PR.

To the original issue. I hide it and paste there

krmanik · 2022-04-17T18:45:41Z

I checked the implementation again I think front/back/styling content addition should be handled in JS API. I will create PR if needed till then this PR does not need any changes.

mikehardy · 2022-04-17T19:58:17Z

Passed local re-integration with current main!

krmanik · 2022-04-18T13:55:53Z

Thanks, three more PR to go

github-actions · 2023-06-23T01:57:32Z

Hi there @krmanik! This is the OpenCollective Notice for PRs merged from 2022-04-01 through 2022-04-30

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please note that GSoC contributions are okay for this process. Our philosophy is that our users have donated to AnkiDroid for all contributions. The only PRs that will not go through the OpenCollective process are ones directly related to am accepted GSoC project from a selected participant, since those receive a stipend from GSoC itself.

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!

Lizzergas reviewed Mar 19, 2022

View reviewed changes

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

krmanik force-pushed the npm-map-json branch from d57294b to aeac254 Compare March 19, 2022 10:30

mikehardy added the Needs Review label Mar 23, 2022

mikehardy requested changes Mar 23, 2022

View reviewed changes

mikehardy added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Mar 23, 2022

krmanik force-pushed the npm-map-json branch 2 times, most recently from 69d83ac to 28fb02b Compare March 24, 2022 05:41

mikehardy requested changes Mar 24, 2022

View reviewed changes

AnkiDroid/src/main/res/values/02-strings.xml Outdated Show resolved Hide resolved

mikehardy approved these changes Mar 24, 2022

View reviewed changes

mikehardy added Needs Second Approval Has one approval, one more approval to merge and removed Needs Author Reply Waiting for a reply from the original author labels Mar 24, 2022

krmanik force-pushed the npm-map-json branch from be3f79c to e7c00b4 Compare March 29, 2022 00:59

david-allison requested changes Mar 29, 2022

View reviewed changes

krmanik force-pushed the npm-map-json branch 3 times, most recently from 07ec4e3 to 444b10d Compare March 30, 2022 14:39

mikehardy requested a review from david-allison April 13, 2022 15:12

david-allison requested changes Apr 15, 2022

View reviewed changes

krmanik added 2 commits April 15, 2022 22:52

added const class for api related constant

a59d0a4

npm utils for checking valid package name

0e4c655

added timber log when validname checks return false

krmanik force-pushed the npm-map-json branch from 444b10d to 6577ff4 Compare April 15, 2022 15:38

david-allison reviewed Apr 15, 2022

View reviewed changes

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

david-allison requested changes Apr 15, 2022

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/jsaddons/AddonData.kt Show resolved Hide resolved

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

david-allison approved these changes Apr 15, 2022

View reviewed changes

david-allison added Needs Author Reply Waiting for a reply from the original author Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Second Approval Has one approval, one more approval to merge labels Apr 15, 2022

get addon model mapped from json

ae8ff13

move test on json file to assets

krmanik force-pushed the npm-map-json branch 2 times, most recently from 0362163 to ff4ed0c Compare April 15, 2022 19:52

david-allison approved these changes Apr 15, 2022

View reviewed changes

david-allison removed the Needs Author Reply Waiting for a reply from the original author label Apr 15, 2022

create new AddonData class and map json then return AddonModel

1961cc5

change data to class and rename AddonModel to AddonData add values to error list change var to val to make it immuatable

krmanik force-pushed the npm-map-json branch from ff4ed0c to 1961cc5 Compare April 15, 2022 20:26

This comment was marked as off-topic.

Sign in to view

mikehardy merged commit 2ba1321 into ankidroid:main Apr 17, 2022

github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Apr 17, 2022

github-actions bot added this to the 2.16 release milestone Apr 17, 2022

krmanik deleted the npm-map-json branch April 18, 2022 13:56

Uh oh!

Conversation

krmanik commented Mar 19, 2022

Pull Request template

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

Uh oh!

Uh oh!

mikehardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikehardy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mikehardy left a comment

Choose a reason for hiding this comment

Uh oh!

mikehardy commented Mar 24, 2022

Uh oh!

krmanik commented Mar 24, 2022

Uh oh!

krmanik commented Mar 29, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikehardy commented Apr 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

krmanik commented Apr 15, 2022

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

david-allison left a comment

Choose a reason for hiding this comment

Uh oh!

mikehardy commented Apr 15, 2022

Uh oh!

mikehardy commented Apr 15, 2022

Uh oh!

krmanik commented Apr 15, 2022

Uh oh!

mikehardy commented Apr 15, 2022

Uh oh!

krmanik commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

david-allison commented Apr 17, 2022

Uh oh!

krmanik commented Apr 17, 2022

Uh oh!

krmanik commented Apr 17, 2022

Uh oh!

mikehardy commented Apr 17, 2022

Uh oh!

krmanik commented Apr 18, 2022

Uh oh!

mikehardy left a comment •

edited

Loading

krmanik commented Apr 15, 2022 •

edited

Loading