coderabbitai[bot]
commented
7 months ago Walkthrough
The updates to the project documentation reflect significant enhancements, focusing on streamlining the setup process and improving test automation. Key changes include upgrading to Python 3.11, detailed instructions for setting up virtual environments, and the integration of system tests within GitHub Actions. Additional updates facilitate user management and data handling, while also supporting financial operations with new scripts and transaction handling features.
Changes
| File | Change Summary |
|---|---|
README.md |
- Upgraded to Python 3.11. - Added virtual environment setup instructions. - Instructions for admin and fake member creation during setup. - Updated local service running instructions. - Added CLI user addition guide. - Integrated system tests in GitHub Actions. - Added Python and JavaScript testing instructions. - Included data wipe warning with make clean-nuke.- New script for Stripe product creation. - Support for exporting transactions as sie files. - Adjusted transaction fee processes in debit bookings. |
Recent Review Details
**Configuration used: CodeRabbit UI** **Review profile: CHILL**Commits
Files that changed from the base of the PR and between 8a511c5cf194eccb0e667b57889246ba43d4db0c and 2eafb0e7d5fa432d2edfffcade1e39a03cd31dbf.Files selected for processing (1)
* README.md (7 hunks)Additional Context Used
LanguageTool (61)
README.md (61)
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 43: Possible spelling mistake found. Context: ...bash python3 -m venv .venv ``` Run the firstrun script ```bash make firstrun ``` This... --- Near line 54: 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 54: 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 56: Possible missing comma found. Context: ...development. If you are deploying on a server you need to configure hosts and other s... --- Near line 57: 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 63: Possible spelling mistake found. Context: ...ker-compose up -d --build ``` ## Start MakerAdmin, web shop, etc. Run all services local... --- Near line 78: Possible spelling mistake found. Context: ...` ### Adding new users that can access MakerAdmin This can be done from the web UI, but ... --- Near line 80: Possible spelling mistake found. Context: ... it can be convenient to do it from the commandline too ``` python3 create_user.py --first... --- Near line 86: Possible spelling mistake found. Context: ...command above you might have to run the commande below to install all dependencies. ```... --- Near line 92: 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 94: Possible spelling mistake found. Context: ...ave (possibly because you have upgraded makeradmin to a newer version) then you might have... --- Near line 95: Possible spelling mistake found. Context: ...ate the permissions. Simply running the firstrun script again will do this: ```bash mak... --- Near line 101: Possible spelling mistake found. Context: ... ```bash make firstrun ``` ### Viewing MakerAdmin etc. Go to: - [the makeradmin web s... --- Near line 105: Possible spelling mistake found. Context: ...ewing MakerAdmin etc. Go to: - [the makeradmin web site](http://localhost:8009) - [t... --- Near line 105: 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 122: Possible spelling mistake found. Context: ...ails to be sent, you'll need to set the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FRO... --- Near line 122: Possible spelling mistake found. Context: ...ou'll need to set the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FROM` variables. You ... --- Near line 122: Possible spelling mistake found. Context: ...the `MAILGUN_DOMAIN`, `MAILGUN_KEY` and `MAILGUN_FROM` variables. You will also want to ... --- Near line 131: Possible spelling mistake found. Context: ...s that requires a running installation Systests are written in python and the sources f... --- Near line 132: Possible spelling mistake found. Context: ...itten in python and the sources for the systests are in the api/src/systest directory (b... --- Near line 132: Possible spelling mistake found. Context: ...the sources for the systests are in the api/src/systest directory (because it share... --- Near line 132: 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 132: Possible spelling mistake found. Context: ...ecause it shares a lot of code with the api unittests). There are tests using the a... --- Near line 132: 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 133: Possible spelling mistake found. Context: ...i unittests). There are tests using the api as well as selenium tests. Those tests ... --- Near line 133: 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 135: 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 147: “you” seems less likely than “your” (belonging to you). Context: ...st your local running environment using you favorite test runner (like pytest). ##... --- Near line 148: Possible spelling mistake found. Context: ...nt using you favorite test runner (like pytest). ### Python unittests The api direct... --- Near line 150: Possible spelling mistake found. Context: ... test runner (like pytest). ### Python unittests The api directory also contains unitte... --- Near line 152: Possible spelling mistake found. Context: ...ike pytest). ### Python unittests The api directory also contains unittests that ... --- Near line 152: Possible spelling mistake found. Context: ...ttests The api directory also contains unittests that can be run standalone, they will a... --- Near line 154: Possible spelling mistake found. Context: ...also run when running `make test`. ### Javascript unit tests Javascript unit tests are r... --- Near line 155: Possible spelling mistake found. Context: ...make test`. ### Javascript unit tests Javascript unit tests are run when the images is b... --- Near line 156: The verb “is” doesn’t seem to fit in this context, “are” is probably more formally correct. Context: ...ript unit tests are run when the images is build but they can also be run against ... --- Near line 156: 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 156: 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 157: 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 157: Possible spelling mistake found. Context: ...rce directly by `make test-admin-js` or `npm --prefix admin run test`. ### If every... --- Near line 161: 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 161: Possible missing comma found. Context: ...ve the existing database and start from scratch you can run the command ``` make clean... --- Near line 167: Possible spelling mistake found. Context: ... this will completely wipe out all your makeradmin data!_ After this you can run `make fi... --- Near line 169: Consider adding a comma here. Context: ...e out all your makeradmin data!_ After this you can run `make firstrun` again to se... --- Near line 169: Possible spelling mistake found. Context: ...keradmin data!_ After this you can run `make firstrun` again to set things up again. ## Deve... --- Near line 169: 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 192: 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 193: Possible spelling mistake found. Context: ...tions will automatically be turned into makeradmin products so that members can purchase t... --- Near line 195: Possible spelling mistake found. Context: ...u should _not_ modify these products in makeradmin. They will be reset whenever the docker... --- Near line 208: Possible spelling mistake found. Context: ...registration page, or the member page), makeradmin will fetch them from stripe and do a bu... --- Near line 212: Possible spelling mistake found. Context: ...ts exporting the transaction history as sie files. The transaction history is comp... --- Near line 214: Possible spelling mistake found. Context: ... and there are no extra transactions in Makeradmin or Stripe. Debit bookings adjust the p... --- Near line 216: “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 216: Possible spelling mistake found. Context: ...ple products. Thus, the transaction fee neds to be distributed over the products if ... --- Near line 216: ‘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 216: 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 216: 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...
Additional comments not posted (7)
README.md (7)
`18-18`: Update Python version to 3.11. Ensure all project dependencies and scripts are compatible with Python 3.11. --- `37-41`: Instructions for setting up a virtual Python environment added. Ensure the provided link to the virtual environment guide remains accessible and up-to-date. --- `54-55`: Added prompts for creating admin users and fake members. Verify that the script correctly handles user inputs and that there are no security concerns with the creation of fake members. --- `79-81`: Added command line support for adding new users. Ensure the script `create_user.py` properly sanitizes inputs to prevent security vulnerabilities. --- `133-133`: Mention of system tests running in GitHub Actions. Confirm that the GitHub Actions workflow is correctly set up to trigger these tests. --- `199-206`: Script for creating required Stripe products included. Ensure the script handles errors gracefully and logs them appropriately for troubleshooting. --- `216-216`: Adjustment process for transaction fees in debit bookings described. Check that the described process is implemented accurately in the code and adheres to financial regulations.
kriffe
Partial solution to #493
Summary by CodeRabbit