Credits and Inventory #905
Conversation
Inventory
|
Test these changes at: https://0185905.pr-review.itflow.org |
|
Unsure why #897 was closed, but would recommend credits and inventory are raised as separate PRs to allow for proper testing. |
|
This is fine for now just for the future one PR per feature will make it easier to manage and review. I see this is still is draft so will review when ready, looking forward to credits and Inventory. |
|
This looks pretty good to me, |
|
On initial review, I'm unable to get the PR Review database past 1.1.0. |
|
@johnnyq Have added a few comments but on mobile atm. Will try and take a look this week once the update queries are working. |
|
@johnnyq When you invited me to fork the repository, that is when we started doing version numbers. |
|
I cannot mark the changes as accepted, but all comments have been resolved. We have now been using for a few days, and have yet to find any quirks. |
|
Something still appears to broken on Database updates. |
There was a problem hiding this comment.
Still struggling to get the update correctly working on my side, syntax issues and mismatch between updated db version and latest db version.
Also looks as though there is a syntax issue in db.sql which would break new installs.
Edit: it would also be great if we could get that Sonar issue count down. Sometimes it's overzealous, but I don't think I've ever seen so many new issues in one PR. Appreciate you're trying to get the bare bones of this working as quickly as possible, which is fine in your fork, but ideally we're aiming for maintainable code here as many others will be relying on it.
| DROP TABLE IF EXISTS `inventory_locations`; | ||
| /*!40101 SET @saved_cs_client = @@character_set_client */; | ||
| /*!40101 SET character_set_client = utf8 */; | ||
| CREATE TABLE `inventory_locations` ( |
There was a problem hiding this comment.
Worth checking with @johnnyq but believe the table name is plural (correct) but the column names should be singular.
https://docs.itflow.org/code_standards#sql_table_structure
Column Naming: Define by non-plural word of the table name separated with an underscore. Example table users has user_id user_first_name
--
Edit: There is a column name mismatch between db.sql and database_updates.php - inventory_location_id VS inventory_locationS_id, etc.
db.sql:
CREATE TABLE `inventory_locations` (
`inventory_locations_id` int(11) NOT NULL AUTO_INCREMENT,
`inventory_locations_name` varchar(200) NOT NULL,
database_updates.php:
mysqli_query($mysqli, "CREATE TABLE `inventory_locations` (
`inventory_location_id` int(11) NOT NULL AUTO_INCREMENT,
`inventory_location_name` varchar(200) NOT NULL,
There was a problem hiding this comment.
yes the field names should be singular good catch
|
There seems to be a few commits missing that must be stashed somewhere. I know I went through and sanitized most if not all of these mentioned. Many thanks for taking the time to recognize that. |
|
Hey @o-psi the field names should be singular for the table inventory_locations to keep consistency |
|
Crazy weekend. This has been fixed. |
Yeah man same here |
|
Credits, and Inventory modules.