Skip to content

com_fields vs cli finder indexer#24630

Merged
HLeithner merged 4 commits intojoomla:stagingfrom
alikon:patch-116
May 28, 2019
Merged

com_fields vs cli finder indexer#24630
HLeithner merged 4 commits intojoomla:stagingfrom
alikon:patch-116

Conversation

@alikon
Copy link
Copy Markdown
Contributor

@alikon alikon commented Apr 17, 2019

Pull Request for Issue #24607

Summary of Changes

always add table search path

Testing Instructions

run finder indexr from cli no warning expected

Expected result

custom fields works as before
and cli finder_indxer runs without warning

Actual result

cli finder_indexer gives warning

cc:@laoneo

thanks @Quy

Co-Authored-By: alikon <optimus4joomla@gmail.com>
@HLeithner
Copy link
Copy Markdown
Member

@jjnxpct can you test this pr?

{
$this->addTablePath(JPATH_ADMINISTRATOR . '/components/com_fields/tables');
}
$this->addTablePath(JPATH_ADMINISTRATOR . '/components/com_fields/tables');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add here a comment why this is necessary as normally you don't have to write this in a model. The environment should be set up properly upfront.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or did you think it is better to add a define with JPATH_COMPONENT in the cli script ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What calls the model? I guess from here and then is the helper here loading the items. I would probably do it in the helper.

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented Apr 18, 2019

I tested this and the error is now gone in the cron email. Excellent.

@ghost
Copy link
Copy Markdown

ghost commented Apr 18, 2019

@jjnxpct please mark your test as successfully at Issue Tracker.

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented Apr 18, 2019

@jjnxpct please mark your test as successfully at Issue Tracker.

@franz-wohlkoenig : Not sure how to do that. Sorry, I'm getting lost in this GitHub stuff...

@ghost
Copy link
Copy Markdown

ghost commented Apr 18, 2019

No Problem @jjnxpct. At https://docs.joomla.org/Testing_Joomla!_patches scroll to Subhead "Recording test results", this might be enough information. If not, please comment :-)

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented Apr 18, 2019

I have tested this item ✅ successfully on 4506625

Tested this on our live site where the problem occurred. After the code change the errors were gone in the cron output mail.


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

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented Apr 18, 2019

@franz-wohlkoenig : Thanks! Just added the successful test.


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

@ghost
Copy link
Copy Markdown

ghost commented Apr 24, 2019

@ReLater can you please test?

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented May 24, 2019

Hi! After updating Joomla the errors came back, because we tested this by changing a core file and now this file has been overriden by the Jomla update.

So I guess this means this has not been added to the Joomla release. Not sure how this works, but when can this 'fix' be expected in a next Joomla release? So we don't have to keep patching this ;-)

@HLeithner
Copy link
Copy Markdown
Member

We need 2 successful tests then it can be merged in to core.

@jjnxpct
Copy link
Copy Markdown

jjnxpct commented May 24, 2019

OK, thanks for letting me know. Hope someone else can test this to.

@Quy
Copy link
Copy Markdown
Contributor

Quy commented May 24, 2019

I have tested this item ✅ successfully on 4e4423e


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

@Quy
Copy link
Copy Markdown
Contributor

Quy commented May 24, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 24, 2019
@HLeithner HLeithner merged commit 9c2f288 into joomla:staging May 28, 2019
@HLeithner HLeithner added this to the Joomla 3.9.7 milestone May 28, 2019
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 28, 2019
@HLeithner
Copy link
Copy Markdown
Member

thx

@alikon alikon deleted the patch-116 branch May 29, 2019 12:01
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