Skip to content

[5.0] Filter values of custom field, fix #42259#42285

Merged
HLeithner merged 3 commits intojoomla:5.0-devfrom
Fedik:field-filter-value
Nov 7, 2023
Merged

[5.0] Filter values of custom field, fix #42259#42285
HLeithner merged 3 commits intojoomla:5.0-devfrom
Fedik:field-filter-value

Conversation

@Fedik
Copy link
Member

@Fedik Fedik commented Nov 4, 2023

Pull Request for Issue #42259 .

Summary of Changes

Filter field value after onCustomFieldsPrepareField

Testing Instructions

Please follow #42259
Create a Text field,
Save it,
And check the field rendering in the layout override.
Or use dump($this->item->jcfields); to see the field ->value

Actual result BEFORE applying this Pull Request

Value contain an empty space

Expected result AFTER applying this Pull Request

Value is empty

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:
  • No documentation changes for docs.joomla.org needed
  • Pull Request link for manual.joomla.org:
  • No documentation changes for manual.joomla.org needed

Reference:

@Fedik Fedik added the bug label Nov 4, 2023
@Fedik Fedik changed the title Filter values of custom field, fix #42259 [5.0] Filter values of custom field, fix #42259 Nov 4, 2023
@Fedik Fedik added the Small A PR which only has a small change label Nov 4, 2023
@Scrabble96
Copy link
Contributor

The com_patchtester_4.3.1 doesn't seem to work in J5 so I can't test this. The error message message says:

An error has occurred while fetching the data from GitHub.

Error inserting pull request data into the database: Incorrect string value: '\xF0\x9F\xA5\x87 )...' for column [mydomain]_joomlalatestversion.m2f0c_patchtester_pulls.description at row 13

@ghost
Copy link

ghost commented Nov 4, 2023

No Problem using Patchtester 4.3.1 in J5.

@Scrabble96
Copy link
Contributor

No Problem using Patchtester 4.3.1 in J5.

Can you confirm what needs to be checked/ticked when I get an access token? Is it just the default that the link from the Patchtester checks? I've installed it on a brand new installation of J5 on a brand new subdomain on a new database.

I uninstalled it, logged out, logged back in and installed it again. Same result.

@ghost
Copy link

ghost commented Nov 4, 2023

pt1
pt2
pt3

@Scrabble96
Copy link
Contributor

All those settings in Joomla are correct. What I meant was, what do I check/tick when I request an authentication token on Github at github.com/settings/tokens? I tried on another test site that had been updated from 4.4.0 to 5.0. Same result. It has public_repo, repo:status selected at the moment.

@ghost
Copy link

ghost commented Nov 4, 2023

pt-token

@angieradtke
Copy link
Contributor

I have tested this item ✅ successfully on 4143e6b

Thanks Fedir, works as expected


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

@HLeithner
Copy link
Member

@Scrabble96 can you please test this pr, simple use the prebuilt packages from https://artifacts.joomla.org/drone/joomla/joomla-cms/5.0-dev/42285/downloads/71682

@korenevskiy
Copy link
Contributor

korenevskiy commented Nov 6, 2023

@Fedik And if you just return the value from the array when checking.

$value = array_filter($value, function ($v) {
  return $v;
});

or

$value = array_filter($value, fn ($v) => $v);

@Fedik
Copy link
Member Author

Fedik commented Nov 6, 2023

@korenevskiy nope, what will hapen when value is '0'?

@korenevskiy
Copy link
Contributor

@korenevskiy nope, what will hapen when value is '0'?

You right!

@Quy
Copy link
Contributor

Quy commented Nov 7, 2023

I have tested this item ✅ successfully on 4143e6b


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

@Quy Quy removed the Small A PR which only has a small change label Nov 7, 2023
@Quy
Copy link
Contributor

Quy commented Nov 7, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 7, 2023
@HLeithner HLeithner merged commit 5acec35 into joomla:5.0-dev Nov 7, 2023
@HLeithner
Copy link
Member

thanks

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 7, 2023
@HLeithner HLeithner added this to the Joomla! 5.0.1 milestone Nov 7, 2023
@angieradtke
Copy link
Contributor

Do we need this Fix in 4.4 as well?

@Fedik Fedik deleted the field-filter-value branch November 7, 2023 08:19
@HLeithner
Copy link
Member

No because we changed the plugin trigger code in 5.0 which leads to this issue.

@angieradtke
Copy link
Contributor

ah ok - understand , thanks

@Scrabble96
Copy link
Contributor

Scrabble96 commented Nov 8, 2023

@Scrabble96 can you please test this pr, simple use the prebuilt packages from https://artifacts.joomla.org/drone/joomla/joomla-cms/5.0-dev/42285/downloads/71682

Hi @HLeithner - I've just got around to testing this. It works fine. Thank you all for fixing and testing this.

@Fedik
Copy link
Member Author

Fedik commented Nov 8, 2023

@laoneo please check my previous comment #42285 (comment)
trim also a bad idea, it will remove "white space" value

@laoneo
Copy link
Member

laoneo commented Nov 8, 2023

Thanks for the info

@Scrabble96
Copy link
Contributor

Silly question, perhaps, but can whatever was done to change it from J4.4.0 to J.5.0 just be reversed, rather than adding yet more code?

@Fedik
Copy link
Member Author

Fedik commented Nov 8, 2023

We have updated old events APi to new 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants