coderabbitai[bot]
commented
2 months ago Walkthrough
The recent updates focus on enhancing system setup processes and member management. These changes introduce new products in the "Medlemskap" category, like "Makerspace base subscription" and "Makerspace access subscription, improving service structuring. Backend improvements include a product ordering generator for consistent ordering and updates to Docker configurations.
Changes
| File | Change Summary |
|---|---|
README.md |
Added new products in the makeradmin category "Medlemskap" to improve member view and registration pages. |
api/src/firstrun.py |
Utilized a generator for consistent product ordering and added a function to create specific Stripe products. |
.github/workflows/makeradmin.yml |
Introduced a firstrun job for environment setup and updated the test job to leverage the latest GitHub Actions features. |
Recent Review Details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**Commits
Files that changed from the base of the PR and between f1bfdea74bd649876ff8113508cbcd6529023d3e and f66d83c14231d73ab4fa295f23ffe210139de24f.Files selected for processing (3)
* .github/workflows/makeradmin.yml (1 hunks) * README.md (1 hunks) * api/src/firstrun.py (8 hunks)Files skipped from review as they are similar to previous changes (2)
* .github/workflows/makeradmin.yml * api/src/firstrun.pyAdditional Context Used
LanguageTool (64)
README.md (64)
Near line 17: Possible spelling mistake found. Context: ...or changes to take effect. ### Python Makeradmin uses Python 3.11. ```bash sudo apt-get... --- Near line 24: The word ‘install’ is not a noun. Context: ...dev python3.10-doc python3-pip ``` The install process will install additional pip pac... --- Near line 25: Possible spelling mistake found. Context: ...all additional pip packages. Activate a venv / virtualenv before install if you want... --- Near line 25: Possible spelling mistake found. Context: ...itional pip packages. Activate a venv / virtualenv before install if you want python envir... --- Near line 27: This sentence does not start with an uppercase letter. Context: ...want python environment isolation. ### npm ```bash sudo apt-get install npm ``` ... --- Near line 44: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing. Context: .... You will also be prompted for if you want to create some fake members, transactions ... --- Near line 44: In American English, abbreviations like “etc.” require a period. Context: ... create some fake members, transactions etc which can be useful for development. I... --- Near line 46: Possible missing comma found. Context: ...development. If you are deploying on a server you need to configure hosts and other s... --- Near line 47: Possible missing comma found. Context: ....env` file. If you do modify the `.env` file you need to restart the services afterw... --- Near line 47: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects. Context: ...` file you need to restart the services afterwards by running ```bash docker-compose up -... --- Near line 53: Possible spelling mistake found. Context: ...ker-compose up -d --build ``` ## Start MakerAdmin, web shop, etc. Run all services local... --- Near line 61: Possible missing comma found. Context: ...h make run ``` You can also run in dev mode where source directories are mounted in... --- Near line 68: Possible spelling mistake found. Context: ...` ### Adding new users that can access MakerAdmin This can be done from the web UI, but ... --- Near line 70: Possible spelling mistake found. Context: ... it can be convenient to do it from the commandline too ``` python3 create_user.py --first... --- Near line 76: Possible missing comma found. Context: ...e admin ``` Before running the command above you might have to run the commande belo... --- Near line 76: Possible spelling mistake found. Context: ...command above you might have to run the commande below to install all dependencies. ```... --- Near line 82: Possible spelling mistake found. Context: ...rmissions for all users to view data on MakerAdmin If the admins don't seem to have the p... --- Near line 84: Possible spelling mistake found. Context: ...ave (possibly because you have upgraded makeradmin to a newer version) then you might have... --- Near line 85: Possible spelling mistake found. Context: ...ate the permissions. Simply running the firstrun script again will do this: ```bash mak... --- Near line 91: Possible spelling mistake found. Context: ... ```bash make firstrun ``` ### Viewing MakerAdmin etc. Go to: - [the makeradmin web s... --- Near line 95: Possible spelling mistake found. Context: ...ewing MakerAdmin etc. Go to: - [the makeradmin web site](http://localhost:8009) - [t... --- Near line 95: Nowadays, it’s more common to write this as one word. Context: ...Admin etc. Go to: - [the makeradmin web site](http://localhost:8009) - [the web sh... --- Near line 112: Possible spelling mistake found. Context: ...ails to be sent, you'll need to set the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FRO... --- Near line 112: Possible spelling mistake found. Context: ...ou'll need to set the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FROM` variables. You ... --- Near line 112: Possible spelling mistake found. Context: ...the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FROM` variables. You will also want to ... --- Near line 121: Possible spelling mistake found. Context: ...s that requires a running installation Systests are written in python and the sources f... --- Near line 122: Possible spelling mistake found. Context: ...itten in python and the sources for the systests are in the api/src/systest directory (b... --- Near line 122: Possible spelling mistake found. Context: ...the sources for the systests are in the api/src/systest directory (because it share... --- Near line 122: Possible spelling mistake found. Context: ...ces for the systests are in the api/src/systest directory (because it shares a lot of c... --- Near line 122: Possible spelling mistake found. Context: ...ecause it shares a lot of code with the api unittests). There are tests using the a... --- Near line 122: Possible spelling mistake found. Context: ...se it shares a lot of code with the api unittests). There are tests using the api as well... --- Near line 123: Possible spelling mistake found. Context: ...i unittests). There are tests using the api as well as selenium tests. Those tests ... --- Near line 123: The official name of this software platform is spelled with a capital “H”. Context: ...nium tests. Those tests are also run in Github actions. You can run the tests in test... --- Near line 125: The noun or adjective “one-off” is spelled with a hyphen. Context: ...un the tests in test containers using a one off db with: ``` make test ``` Or you can... --- Near line 137: “you” seems less likely than “your” (belonging to you). Context: ...st your local running environment using you favorite test runner (like pytest). ##... --- Near line 138: Possible spelling mistake found. Context: ...nt using you favorite test runner (like pytest). ### Python unittests The api direct... --- Near line 140: Possible spelling mistake found. Context: ... test runner (like pytest). ### Python unittests The api directory also contains unitte... --- Near line 142: Possible spelling mistake found. Context: ...ike pytest). ### Python unittests The api directory also contains unittests that ... --- Near line 142: Possible spelling mistake found. Context: ...ttests The api directory also contains unittests that can be run standalone, they will a... --- Near line 144: Possible spelling mistake found. Context: ...also run when running `make test`. ### Javascript unit tests Javascript unit tests are r... --- Near line 145: Possible spelling mistake found. Context: ...make test`. ### Javascript unit tests Javascript unit tests are run when the images is b... --- Near line 146: “images” is a plural noun. It appears that the verb form is incorrect. Context: ...ript unit tests are run when the images is build but they can also be run against ... --- Near line 146: Make sure that the noun ‘build’ is correct. Did you mean the past participle “built”? Context: ...t unit tests are run when the images is build but they can also be run against the so... --- Near line 146: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short). Context: ...t tests are run when the images is build but they can also be run against the source... --- Near line 147: Possible spelling mistake found. Context: ...o be run against the source directly by `make test-admin-js` or `npm --prefix admin run test`. ###... --- Near line 147: Possible spelling mistake found. Context: ...rce directly by `make test-admin-js` or `npm --prefix admin run test`. ### If every... --- Near line 151: Consider wrapping this idiom with two commas if you want to put a strong emphasize on it. Context: ...`. ### If everything goes wrong If you for some reason want to remove the existing database an... --- Near line 157: Possible spelling mistake found. Context: ... this will completely wipe out all your makeradmin data!_ After this you can run `make fi... --- Near line 159: Consider adding a comma here. Context: ...e out all your makeradmin data!_ After this you can run `make firstrun` again to se... --- Near line 159: Possible spelling mistake found. Context: ...keradmin data!_ After this you can run `make firstrun` again to set things up again. ## Deve... --- Near line 159: This sentence contains multiple usages of the word “again”. Consider removing or replacing it. Context: ... `make firstrun` again to set things up again. ## Development with Stripe Create yo... --- Near line 182: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing. Context: ...ducts When using stripe, subscriptions need to be configured via the stripe website. T... --- Near line 183: Possible spelling mistake found. Context: ...tions will automatically be turned into makeradmin products so that members can purchase t... --- Near line 185: Possible spelling mistake found. Context: ...u should _not_ modify these products in makeradmin. They will be reset whenever the docker... --- Near line 198: Possible spelling mistake found. Context: ...registration page, or the member page), makeradmin will fetch them from stripe and do a bu... --- Near line 202: Possible missing comma found. Context: ...em. ## Bookkeeping and accounting For accounting Makeradmin supports exporting the trans... --- Near line 202: Possible spelling mistake found. Context: ...ts exporting the transaction history as sie files. The transaction history is comp... --- Near line 204: Possible missing comma found. Context: ...is compared with Stripe to make sure it matches and there are no extra transactions in ... --- Near line 204: Possible spelling mistake found. Context: ... and there are no extra transactions in Makeradmin or Stripe. Debit bookings adjust the p... --- Near line 206: “Transaction” is a singular noun. It appears that the verb form is incorrect. Context: ... fee. However, each transaction usually contain multiple products. Thus, the transactio... --- Near line 206: Possible spelling mistake found. Context: ...ple products. Thus, the transaction fee neds to be distributed over the products if ... --- Near line 206: ‘in proportion to’ might be wordy. Consider a shorter alternative. Context: ... the transaction fee among the products in proportion to their total price within the transactio... --- Near line 206: The phrase “takes into consideration” may be wordy. To make your writing clearer, consider replacing it. Context: ...tal price within the transaction, which takes into consideration the quantity of products purchased. Any... --- Near line 206: The noun “leftover” is spelled as one word. Context: ...e nearest, higher or lower, amount. Any left over cent is added or removed on the most ex...
emanuelen5
Summary by CodeRabbit