Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[com_fields] Remove unneeded fields from com_fields #13621

Merged
merged 2 commits into from
Jan 23, 2017

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Jan 17, 2017

Pull Request for Issue #12733.

Summary of Changes

  • Removes image and image alternate text fields from the field definition. Those aren't used anywhere and would only confuse users.
  • Removes options, attributes, created_by_alias, publish_up and publish_down columns from the #__fields table. They are not used at all and make no sense in the context of a field.

Testing Instructions

  • Test that creating, editing and general use of fields work as before. There should be no difference except that the two "image" and "image alternate text" fields are no longer part of the edit form.
  • To test the database changes, you need to either install a fresh installation or update from a 3.6.5 installation to this branch. So this may be a bit tricky. You can however also manually delete the columns and test or just create a few fields and verify there is no data stored into those columns.

Documentation Changes Required

None

@Bakual Bakual changed the title Remove unneeded fields from com_fields [com_fields] Remove unneeded fields from com_fields Jan 17, 2017
@laoneo
Copy link
Member

laoneo commented Jan 17, 2017

The update scripts are missing.

@zero-24
Copy link
Contributor

zero-24 commented Jan 17, 2017

The update scripts are missing.

@laoneo There are no update needed as it was not in any stable release. ;)

@zero-24 zero-24 added this to the Joomla 3.7.0 milestone Jan 17, 2017
@Bakual
Copy link
Contributor Author

Bakual commented Jan 17, 2017

The update scripts are missing.

Look again. The update scripts are there. I adjusted the existing ones so we don't add the fields and remove them again.

@zero-24
Copy link
Contributor

zero-24 commented Jan 17, 2017

@laoneo
Copy link
Member

laoneo commented Jan 17, 2017

Was a mistake on my side. Sorry for the confusion, too long day.

@ghost
Copy link

ghost commented Jan 19, 2017

I have tested this item 🔴 unsuccessfully on 5fd022a

Tested 2 Fields in Field Group "Details":

  • Field "Colour" is shown correct below "Details"
  • Field "Editor" is shown at "Basic Settings".

bildschirmfoto 2017-01-19 um 20 59 00
Test on fresh installed 3.6.5 + update on 3.7.0-beta1


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13621.

@Bakual
Copy link
Contributor Author

Bakual commented Jan 19, 2017

@franz-wohlkoenig That's an interesting bug you found there, but not related to this PR.
I actually suspected that this behavior could be possible but haven't got the time to try to produce it. What happens here is that if you name the field alias the same as an existing regular formfield, the custom one will overwrite the existing one. There are two reasons for that: First we allow the admin to name the field the way he wants, which means it can take the name of an existing one. Second we're using the "params" form group to store our fields. That group often is already used by the original form. So it clashes there.

There are already PRs open which would solve that issue:

@ghost
Copy link

ghost commented Jan 19, 2017

@Bakual Changing Fieldname from Editor to Editor1 solved Problem:
bildschirmfoto 2017-01-19 um 21 57 35

@ghost
Copy link

ghost commented Jan 21, 2017

Tested Field-type Gallery on Article got on different Paths in Front- and Backend:

  • Warning: JFolder: :files: Path is not a folder. Path: __/images/images/sampledata/fruitshop
  • and Picture-Gallery: Warning: Invalid argument supplied for foreach() in /plugins/fields/gallery/tmpl/gallery.php on line 50

@Bakual
Copy link
Contributor Author

Bakual commented Jan 21, 2017

@franz-wohlkoenig Is that on a Windows machine?

It isn't an issue of this PR, but may be a bug related to the different directory separators.

@ghost
Copy link

ghost commented Jan 21, 2017

Test on:
Joomla! 3.7.0-beta1-nightly
macOS Sierra, 10.12.2
Firefox 50.1.0
PHP 7.0.4
MySQLi 5.5.53-0
See #13706

@laoneo
Copy link
Member

laoneo commented Jan 22, 2017

I have tested this item ✅ successfully on 5fd022a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13621.

1 similar comment
@chmst
Copy link
Contributor

chmst commented Jan 23, 2017

I have tested this item ✅ successfully on 5fd022a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13621.

@astridx
Copy link
Contributor

astridx commented Jan 23, 2017

I have tested this item ✅ successfully on 5fd022a

1. I pull the current version of joomla-cms
2. I made a new installation without testdata
3. I installed Patch tester but I get the message
1146 Table 'joomla-cms.#__patchtester_pulls' doesn't exist
I solved this problem by creating the database tables manually. I had this problem earlier, that is why I tell it. Perhaps someone has a hint for me.
4. I applied the patch
5. I deleted the configuration.php and copied a installation folder and made new installation.
6. I created a lot of field and everything worked well.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13621.

1 similar comment
@astridx
Copy link
Contributor

astridx commented Jan 23, 2017

I have tested this item ✅ successfully on 5fd022a

1. I pull the current version of joomla-cms
2. I made a new installation without testdata
3. I installed Patch tester but I get the message
1146 Table 'joomla-cms.#__patchtester_pulls' doesn't exist
I solved this problem by creating the database tables manually. I had this problem earlier, that is why I tell it. Perhaps someone has a hint for me.
4. I applied the patch
5. I deleted the configuration.php and copied a installation folder and made new installation.
6. I created a lot of field and everything worked well.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13621.

@rdeutz rdeutz merged commit c437fce into joomla:staging Jan 23, 2017
@Bakual Bakual deleted the RemoveUnneededFields branch January 23, 2017 20:30
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.

7 participants