Skip to content

Conversation

@SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Feb 20, 2020

Summary of Changes

This updates entries in #__content_types table to include new fully qualified table class names.
Components that use JTable prefixed core classes are currently not included but maybe they should be. Looking for feedback on this.

Testing Instructions

This will need to be tested both for new installations and for upgrades from 3.10.
Create a contact, a newsfeed, a tag, a banner, a banner client and a user note.
Make some edits and save.
In the form, click Versions button and check whether the current version is marked with a star.

Expected result

Marked.

Actual result

Not marked.

Documentation Changes Required

No.

@richard67
Copy link
Member

@SharkyKZ In the update sql script for postgresql in the alter table statements you can't use varchar, you have to use character varying because our database checker is so silly. In create table statements in update sql it doesn't mater because for these it doesn't check particular columns. See 4.0.0-2018-07-29.sql or 4.0.0-2019-05-20.sql for examples on usage of character varying.

@astridx
Copy link
Contributor

astridx commented Feb 23, 2020

I have tested this

  1. git fetch origin pull/27992/head:j4/ucm/getContentTable
  2. git checkout j4/ucm/getContentTable
  3. I made a new installation
  4. npm or composer is not needed.
  5. First I checked my mysql database. The changes are made.

new.txt
old.txt

6. Then I created a Tag, edited the Tag. But I could not see a Star. I think it is because the Table is not named TagsTable. The class is named TagTable (without the s),
7. I could see the star when I tried it with a Banner, a Contact and a Newsfeed.

I tested with the current ‎4.0.0-beta1-dev on Ubuntu with PHP 7.2 and MySQL 5.7.

@SharkyKZ
Copy link
Contributor Author

@astridx Should be fixed now.

@wilsonge wilsonge merged commit 45b8156 into joomla:4.0-dev Mar 12, 2020
@wilsonge
Copy link
Contributor

Thanks!

@wilsonge
Copy link
Contributor

wilsonge commented Mar 12, 2020

Components that use JTable prefixed core classes are currently not included but maybe they should be. Looking for feedback on this.

I think they should be

@SharkyKZ
Copy link
Contributor Author

Should we use the core classes or classes from components, e.g. Joomla\CMS\Table\Content or Joomla\Component\Content\Administrator\Table\ArticleTable. Asking this because I see both used throughout the CMS and not sure which one is the right way.

@wilsonge
Copy link
Contributor

If there’s a component specific one use it.

I think I want to end up with components managing these things long terms anyhow so we can use the bootComponent methods for access anyhow.

@zero-24 zero-24 added this to the Joomla 4.0 milestone Mar 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants