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.

Sign up for GitHub

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

Jump to bottom

kolibri_explore_plugin: Make various UI strings translatable #782

Merged
merged 1 commit into from
Sep 19, 2023
Merged

kolibri_explore_plugin: Make various UI strings translatable #782

merged 1 commit into from
Sep 19, 2023

Conversation

pwithnall
Copy link
Contributor

Signed-off-by: Philip Withnall [email protected]

Fixes: #773

@pwithnall pwithnall self-assigned this Sep 5, 2023
@pwithnall
Copy link
Contributor Author

This is based on #777, so that needs to be reviewed and merged before this one. Only the top commit in this PR is actually specific to kolibri_explore_plugin core.

@pwithnall pwithnall force-pushed the 773-kolibri-explore-plugin-translation branch from 269b073 to d1d9e2e Compare September 5, 2023 18:50
@pwithnall
Copy link
Contributor Author

Rebasing this now that #777 has landed.

@pwithnall pwithnall force-pushed the 773-kolibri-explore-plugin-translation branch from d1d9e2e to cb64018 Compare September 18, 2023 13:06
@pwithnall
Copy link
Contributor Author

Pushing to test CI. It is failing to build locally for me but I can’t work out why (and so is master), so I think it might be an environment problem for me.

@pwithnall
Copy link
Contributor Author

So the CI is failing, but with a different failure from what I get locally, neither of which occur on master, and this PR doesn’t touch anything to do with the build system.

I guess I need to dig deeper.

@pwithnall pwithnall marked this pull request as draft September 18, 2023 13:14
@manuq
Copy link
Collaborator

manuq commented Sep 18, 2023

@pwithnall running the linter locally succeeds on my end. The CI says error kolibri-explore-plugin@: The engine "node" is incompatible with this module. Expected version "16.x". Got "18.17.1" . Since when the CI is running node v18? I have v16 here and that's what the scripts/bootstrap.sh ensures too.

@manuq
Copy link
Collaborator

manuq commented Sep 18, 2023

@dbnicholson maybe the changes in a06ac87 (which are very well intentioned) regressed the node version in the CI.

@pwithnall
Copy link
Contributor Author

Locally I’m getting the following (and am also getting it on master), and I haven’t been able to make headway on understanding it yet:

Kolibri Build: Building 'kolibri_explore_plugin.app' bundle...
Kolibri Build: Building 'kolibri_explore_plugin.side_nav' bundle...
Kolibri Build: Completed 'kolibri_explore_plugin.side_nav' bundle in 2.685s!
[BABEL] Note: The code generator has deoptimised the styling of /home/philip/Projects/kolibri-explore-plugin/node_modules/bootstrap-vue/esm/icons/icons.js as it exceeds the max of 500KB.
[BABEL] Note: The code generator has deoptimised the styling of /home/philip/Projects/kolibri-explore-plugin/node_modules/lodash/lodash.js as it exceeds the max of 500KB.
Kolibri Build: Failed to compile 'kolibri_explore_plugin.app' bundle!
kolibri_explore_plugin.app6.40.1.dev12+g0f97ae2.css from Css Minimizer plugin
TypeError: Cannot read properties of undefined (reading 'Universal')

Kolibri Build: Build complete in 34.315 seconds
Kolibri Build: kolibri_explore_plugin.app:
  ERROR in kolibri_explore_plugin.app6.40.1.dev12+g0f97ae2.css
  kolibri_explore_plugin.app6.40.1.dev12+g0f97ae2.css from Css Minimizer plugin
  TypeError: Cannot read properties of undefined (reading 'Universal')
      at Object.<anonymous> (/home/philip/Projects/kolibri/packages/kolibri-tools/node_modules/css-select/lib/sort.js:6:30)
      at Module._compile (node:internal/modules/cjs/loader:1103:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
      at Module.load (node:internal/modules/cjs/loader:981:32)
      at Function.Module._load (node:internal/modules/cjs/loader:822:12)
      at Module.require (node:internal/modules/cjs/loader:1005:19)
      at require (node:internal/modules/cjs/helpers:102:18)
      at Object.<anonymous> (/home/philip/Projects/kolibri/packages/kolibri-tools/node_modules/css-select/lib/compile.js:32:30)
      at Module._compile (node:internal/modules/cjs/loader:1103:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)

  ERROR in kolibri_explore_plugin.app6.40.1.dev12+g0f97ae2.rtl.css
  kolibri_explore_plugin.app6.40.1.dev12+g0f97ae2.rtl.css from Css Minimizer plugin
  TypeError: Cannot read properties of undefined (reading 'Universal')
      at Object.<anonymous> (/home/philip/Projects/kolibri/packages/kolibri-tools/node_modules/css-select/lib/sort.js:6:30)
      at Module._compile (node:internal/modules/cjs/loader:1103:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)
      at Module.load (node:internal/modules/cjs/loader:981:32)
      at Function.Module._load (node:internal/modules/cjs/loader:822:12)
      at Module.require (node:internal/modules/cjs/loader:1005:19)
      at require (node:internal/modules/cjs/helpers:102:18)
      at Object.<anonymous> (/home/philip/Projects/kolibri/packages/kolibri-tools/node_modules/css-select/lib/compile.js:32:30)
      at Module._compile (node:internal/modules/cjs/loader:1103:14)
      at Object.Module._extensions..js (node:internal/modules/cjs/loader:1155:10)

  kolibri_explore_plugin.app compiled with 2 errors
error Command failed with exit code 1.

@dbnicholson
Copy link
Member

Gah! Yes, nodeenv is failing:

* Install prebuilt node (16.14.0) .Traceback (most recent call last):
  File "/home/runner/.local/share/virtualenvs/kolibri-explore-plugin-eWLv29zv/bin/nodeenv", line 8, in <module>
    sys.exit(main())
  File "/home/runner/.local/share/virtualenvs/kolibri-explore-plugin-eWLv29zv/lib/python3.10/site-packages/nodeenv.py", line 1076, in main
    create_environment(env_dir, opt)
  File "/home/runner/.local/share/virtualenvs/kolibri-explore-plugin-eWLv29zv/lib/python3.10/site-packages/nodeenv.py", line 905, in create_environment
    install_node(env_dir, src_dir, opt)
  File "/home/runner/.local/share/virtualenvs/kolibri-explore-plugin-eWLv29zv/lib/python3.10/site-packages/nodeenv.py", line 692, in install_node
    download_node_src(node_url, src_dir, opt, prefix)
  File "/home/runner/.local/share/virtualenvs/kolibri-explore-plugin-eWLv29zv/lib/python3.10/site-packages/nodeenv.py", line 542, in download_node_src
    dl_contents = io.BytesIO(urlopen(node_url).read())
  File "/usr/lib/python3.10/http/client.py", line 482, in read
    s = self._safe_read(self.length)
  File "/usr/lib/python3.10/http/client.py", line 633, in _safe_read
    raise IncompleteRead(data, amt-len(data))
http.client.IncompleteRead: IncompleteRead(9191124 bytes read, 2362[28](https://github.com/endlessm/kolibri-explore-plugin/actions/runs/6222957258/job/16887894393?pr=782#step:8:29)[29](https://github.com/endlessm/kolibri-explore-plugin/actions/runs/6222957258/job/16887894393?pr=782#step:8:30) more expected)

However, since bootstrap.sh is run as bash ./scripts/bootstrap.sh, it overrides the -e in the #!/bin/bash -e shebang line. It would be better to move the set -e below the shebang and/or just run the script directly as ./scripts/bootstrap.sh in the workflows, but that won't change the fact that nodeenv is failing.

@dbnicholson
Copy link
Member

I might be wrong, but I think this is ekalinin/nodeenv#324, which is ultimately nodejs/build#1993.

Well, a more "proper" way to fix it would be to add back the setup_node action and add a switch to bootstrap.sh that explicitly skips running nodeenv. I don't think that the current way where errors from bootstrap.sh are silently ignored are a good idea. I can work on that in a bit, but I have something to do this morning.

@pwithnall
Copy link
Contributor Author

Well, a more "proper" way to fix it would be to add back the setup_node action and add a switch to bootstrap.sh that explicitly skips running nodeenv.

I’ll look at doing that now, since I’m getting absolutely nowhere with this ‘Css Minimizer’ failure.

@pwithnall pwithnall mentioned this pull request Sep 18, 2023
Merged
@pwithnall pwithnall force-pushed the 773-kolibri-explore-plugin-translation branch from cb64018 to f33396f Compare September 18, 2023 15:14
@pwithnall
Copy link
Contributor Author

Looks like my local issue with ‘Css Minimizer’ was something to do with having yarn linked in kolibri-tools. Unlinking has fixed things locally for me. I’m not quite sure what the difference was, as my local kolibri-tools checkout is pretty close to 0.16.0-dev.4, but kolibri-tools has bumped some babel/vue dependencies recently. 🤷

Anyway, now I can actually test these changes.

@pwithnall
kolibri_explore_plugin: Make various UI strings translatable
674ceed 
Note that the `supportBody` string contains inline HTML which has to be
embedded in the translation, so it’s inserted into the template using
`v-html`. This is a potential injection site for malicious data if
anything other than developer-controlled translated strings are used
there. So don’t do that.

Signed-off-by: Philip Withnall <[email protected]>

Fixes: #773
@pwithnall pwithnall force-pushed the 773-kolibri-explore-plugin-translation branch from f33396f to 674ceed Compare September 18, 2023 15:45
@pwithnall
Copy link
Contributor Author

OK, I think this is ready for review now. The only change since last time anyone saw it was that I’ve had to use v-html to insert the supportBody string, as it contains inline markup which would otherwise be escaped. The use of v-html could be an injection vector if we’re not careful to make sure it is only ever passed developer-controlled data, so it could do with some close review.

@pwithnall pwithnall marked this pull request as ready for review September 18, 2023 15:49
@pwithnall
Copy link
Contributor Author

Merging as per approval

@pwithnall pwithnall merged commit 33c84ce into master Sep 19, 2023
@pwithnall pwithnall deleted the 773-kolibri-explore-plugin-translation branch September 19, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manuq manuq manuq approved these changes

@dylanmccall dylanmccall Awaiting requested review from dylanmccall

Assignees

@pwithnall pwithnall

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add translation calls to kolibri_explore_plugin
3 participants
@pwithnall @manuq @dbnicholson