? ? Pending

User tests: Successful: Unsuccessful:

avatar wilsonge
wilsonge
8 Apr 2020

Summary of Changes

Attempts to add some foreign keys into Joomla. Starts with the usergroup mapping table as a POC as it's a fairly important table where we shouldn't leave any bad data around

I think the schema checker will need more work. Currently it relies on the constaint name being unique in the db. It's not totally unreasonable to assume this - especially in an application like Joomla (i've never come across a case where it's no unique inside and outside of Joomla). But it can't hurt to improve it either over time.

Why do we need this?

Currently we largely rely on PHP to do our data sanity checks however other extensions can and do insert stuff into the database - sometimes without performing adequate security checks.

This enforces some of these validations at the database level. The user group mapping has to point to a valid user in the database and valid group in the groups tables. If you try and insert the data directly into the database with id's that don't exist in either of these columns then it will fail (even if you use a tool like phpmyadmin)

Testing Instructions

Check you can still assign groups to users. Check database schema checker doesn't error. Note that the schema checker can only validate the keys exist on DBs where there is root access to the db (due to how mysql stores foreign keys). If you have permissions it will make the check. If you don't then it will mark the check as skipped (this is just straightforward feature enhancement).

@alikon @richard67 please check the postgres implementation. It's based off google search results. I believe it will work fine regardless of permissions as postgres stores constraints differently

Documentation Changes Required

Yup. And @zero-24 we might want to add some validation for the things we add foreign keys for in 3.10

avatar wilsonge wilsonge - open - 8 Apr 2020
avatar wilsonge wilsonge - change - 8 Apr 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 8 Apr 2020
Category SQL Administration com_admin Postgresql Language & Strings Installation Libraries
avatar wilsonge wilsonge - change - 8 Apr 2020
Labels Added: ? ?
avatar wilsonge wilsonge - change - 9 Apr 2020
The description was changed
avatar wilsonge wilsonge - edited - 9 Apr 2020
avatar wilsonge wilsonge - change - 9 Apr 2020
The description was changed
avatar wilsonge wilsonge - edited - 9 Apr 2020
avatar alikon
alikon - comment - 12 Apr 2020

at first glance i got this one on postgresql
Screenshot from 2020-04-12 08-53-47

avatar richard67
richard67 - comment - 12 Apr 2020

@alikon Well, George asked us to check the PostgreSQL part, so if you can propose a fix, do so.

avatar alikon
alikon - comment - 12 Apr 2020
avatar richard67
richard67 - comment - 12 Apr 2020

@alikon wilsonge#49 is fixing it but not in a consistent way, see my comment there. I'd prefer just to fix the update sql to use integer instead of INTEGER.

avatar richard67
richard67 - comment - 12 Apr 2020

wilsonge#49 is ok now after last correction.

avatar wilsonge wilsonge - change - 30 May 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-05-30 14:27:21
Closed_By wilsonge
avatar wilsonge wilsonge - close - 30 May 2020
avatar wilsonge wilsonge - merge - 30 May 2020
avatar wilsonge
wilsonge - comment - 30 May 2020

Cheers for reviewing this guys!

Add a Comment

Login with GitHub to post a comment