[4.0] Fixing Articles batch copy by infograf768 · Pull Request #26835 · joomla/joomla-cms

infograf768 · 2019-10-27T06:04:50Z

Summary of Changes

Adapting code to the new columns featured_up and featured_down

Testing Instructions

Select a featured or a non-featured article and batch copy it to another category.

Before patch

Fails

After patch

All is fine

Documentation Changes Required

alikon · 2019-10-27T08:19:31Z

article copy to another category don't work, even with non featured article
and i'm testing with mysql

richard67 · 2019-10-27T08:20:44Z

Yes, just tested: With this PR the error goes away and success message is shown, but then I can't see any copied article.
Update: Can't see because it doesn't exist. Copying failed.

alikon · 2019-10-27T08:22:18Z

even if the article is present in the #__content table
maybe workflow ???

richard67 · 2019-10-27T08:27:34Z

Yes, in DB they exist, the copied ones have state=0. But then I set filter to show also trashed items I don't see them.

richard67 · 2019-10-27T08:28:31Z

No PHP warning or notice or error, nothing in MySQL log.

brianteeman · 2019-10-27T08:28:32Z

almost certainly workflow related.

richard67 · 2019-10-27T08:29:07Z

You can strike through the "almost" I would say.

brianteeman · 2019-10-27T08:30:33Z

i was being polite

alikon · 2019-10-27T08:30:53Z

politically incorrect comment "i hate workflow" 😄

richard67 · 2019-10-27T08:34:54Z

Question is how we shall coninue with this PR here? It fixes the error "1136 Column count doesn't match value count at row 1" when batch copying a featured article. The general problem we've discovered here, should that be handled in a separate issue?

alikon · 2019-10-27T08:38:02Z

pr itself is correct imho
anyway the copy issue is workflow related

infograf768 · 2019-10-27T08:43:55Z

The general problem we've discovered here, should that be handled in a separate issue?

We can separate if we merge this fast. Otherwise It can be closed and included in the full patch.

richard67 · 2019-10-27T08:45:28Z

I have tested this item ✅ successfully on 22a19e7

Fixes the error "1136 Column count doesn't match value count at row 1" when batch copying a featured article.

Other problems with batch processing are out of scope of this PR.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26835.}

infograf768 · 2019-10-27T08:46:56Z

Modified title to fit.

richard67 · 2019-10-27T08:48:53Z

@alikon My query shows an additional condition:

WHERE ws.condition IN (:preparedArray1,:preparedArray2) AND wa.extension='com_content'

Could this one be the problem? Unfortunately we don't see values of prepared statements in debug.

SharkyKZ · 2019-10-27T08:51:54Z

administrator/components/com_content/Model/ArticleModel.php

 			$query = $db->getQuery(true)
 				->insert($db->quoteName('#__content_frontpage'))
-				->values($newId . ', 0');
+				->values($newId . ', 0, NULL, NULL');


These should be dates from copied article.

Not sure. Dates have to fit to copied state. If featured flag is reset, too, and status, then also these have to be reset. It needs to check what the '0' nefore these values is, but I don't see the columns list here.

@alikon Check the file ... an insert without a columns list ... that's a kind of lotto play. Am I right, or do I miss something?

don't get you

@alikon When i do an insert statement, i have a columns list, so it reads "INSERT INTO blabla_table (col1, col2, col3) VALUES (1,2,3);". Here in this statement I don't see that columns list, only something wihc result in "INSERT INTO blabla_table VALUES (1,2,3);"

Legit but not safe in case of schema changes

that's another story... you'll allways have to do things carefully when a schema change happen, even with the full column name syntax...

@SharkyKZ I think you are right, but for this I think it needs to move up lines 96 $oldItem = $this->getTable(); and 97 $oldItem->load($oldId); up to before the insert statement, right? The load routine of that table btw. is the one which I thought I could remove with PR #26829 , good that I haven't done this. It loads the featured up and down times from the content frontpage table and copies them into the article table. These then should be used here instead of the NULLs. Please confirm if that would be the right way.

@infograf768 If that is right, should I make a PR against your branch to save you some work, so you just can merge it into this PR?

hmm... Looks like you are right.

I have it here, Shall I make PR for you, @infograf768 . Is not as complicated as I wrote above. But it will not solve our other, more general problem, only will make the times being copied right.

alikon · 2019-10-27T08:56:34Z

AFAIK com_content has not been converted to prepared statement.... maybe some MVC libraries...

richard67 · 2019-10-27T10:32:46Z

@infograf768 Please check infograf768#54. It implements @SharkyKZ 's comment that the dates of the copied article shall be used. Unfortunately it does not solve the other, more global problem.

infograf768 · 2019-10-27T11:58:46Z

@richard67 See my comment in your PR

anibalsanchez · 2019-10-27T15:36:12Z

I have tested this item 🔴 unsuccessfully on 22a19e7

Everything works normally, and the new article is added to the content and content_froentend table. But, the new article is not shown in the list.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26835.}

[4.0] Use featured up and down times from copied article, if set

infograf768 · 2019-10-28T09:06:28Z

@anibalsanchez
The PR does not solve fully Batch Copy, it only solves the error due to wrong columns for featured_up and featured_down.

Solving fully batch copy will require another PR.

richard67 · 2019-10-28T09:07:19Z

@anibalsanchez Please test again with respect to the comment in my previous test result above:

Fixes the error "1136 Column count doesn't match value count at row 1" when batch copying a featured article.

Other problems with batch processing are out of scope of this PR.

richard67 · 2019-11-12T18:10:45Z

@infograf768 As @wilsonge commented here infograf768#55 (comment), my PR infograf768#55 seems to be ok. Do you want to merge it into this PR so it can be tested?

alikon · 2019-11-12T19:51:17Z

even if, apart wilsonge thinking,
i second this
😄

wilsonge · 2019-11-12T21:45:53Z

I haven’t tested it but if it works it’s definitely the better fix to use the post install cleanup and the built in library

richard67 · 2019-11-12T21:53:57Z

Post install cleanup?

richard67 · 2019-11-12T21:59:23Z

How is the post install cleanup related to this here?

wilsonge · 2019-11-12T22:06:26Z

I meant the cleanupPostBatchCopy method :)

richard67 · 2019-11-12T22:07:34Z

Ah now I understand.

richard67 · 2019-11-12T22:08:44Z

Was confused because there is also a cleanup of post install messages issue ;-)

infograf768 · 2019-11-13T09:21:43Z

Where do we stand here?

richard67 · 2019-11-13T10:23:00Z

@infograf768 As said above: Merge my PR for you, change title and description of this PR here so it‘s clear that it solves the issue with batch copying articles in general, also if not featured, then refer to this PR here in Brian‘s issue and finally wait for testers.

…-workflow-assocs [4.0] Batch copy workflow association, too

infograf768 · 2019-11-13T10:25:56Z

@richard67
Done.

richard67 · 2019-11-13T10:42:40Z

@infograf768 Now it needs to change title and description of this PR so it is clear that it solves the issue with batch copying articles in general, also if not featured. I can't do this for you now because am at work. I'll leave a reference to this PR in Brian's issue.

richard67 · 2019-11-13T10:48:15Z

I've left a reference comment in PR #24519 that this PR here does the same but in a mroe elegant way.

I've also left comments in issues #24478 and #27060 that this PR here solves these issues.

infograf768 · 2019-11-14T06:47:23Z

@SharkyKZ @alikon
Please test again.

SharkyKZ · 2019-11-14T07:59:34Z

I have tested this item ✅ successfully on fc0d70a

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26835.}

Quy · 2019-11-14T15:45:15Z

I have tested this item ✅ successfully on fc0d70a

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26835.}

Quy · 2019-11-14T15:45:24Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26835.}

wilsonge · 2019-11-14T16:16:24Z

Thanks!

[4.0] Fixing Articles batch copy

dba0fbb

joomla-cms-bot added the PR-4.0-dev label Oct 27, 2019

Merge branch '4.0-dev' into 4.0_batch_copy_article

22a19e7

infograf768 changed the title ~~[4.0] Fixing Featured Articles batch copy~~ [4.0] Fixing Featured Articles batch copy (featured aspects) Oct 27, 2019

SharkyKZ reviewed Oct 27, 2019

View reviewed changes

Use featured up and down times from copied article

9832a69

richard67 mentioned this pull request Oct 27, 2019

[4.0] Use featured up and down times from copied article, if set infograf768/joomla-cms#54

Merged

Merge pull request #54 from richard67/patch-1

1dccadb

[4.0] Use featured up and down times from copied article, if set

Merge branch '4.0-dev' into 4.0_batch_copy_article

bff961e

richard67 mentioned this pull request Nov 12, 2019

[4.0] batch content fails #27060

Closed

Merge pull request #55 from richard67/4.0-dev-fix-articles-batch-copy…

6c6ef3d

…-workflow-assocs [4.0] Batch copy workflow association, too

richard67 mentioned this pull request Nov 13, 2019

[4.0] Fix batch copy for articles #24519

Closed

infograf768 changed the title ~~[4.0] Fixing Featured Articles batch copy (featured aspects)~~ [4.0] Fixing Articles batch copy Nov 13, 2019

richard67 mentioned this pull request Nov 13, 2019

[4.0] Batch copy fails #24478

Closed

Merge branch '4.0-dev' into 4.0_batch_copy_article

3979966

Merge branch '4.0-dev' into 4.0_batch_copy_article

fc0d70a

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Nov 14, 2019

wilsonge merged commit 4d6c679 into joomla:4.0-dev Nov 14, 2019

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Nov 14, 2019

wilsonge added this to the Joomla 4.0 milestone Nov 14, 2019

infograf768 deleted the 4.0_batch_copy_article branch November 16, 2019 07:58

Uh oh!

Conversation

infograf768 commented Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Testing Instructions

Before patch

After patch

Documentation Changes Required

Uh oh!

alikon commented Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richard67 commented Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alikon commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

brianteeman commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

brianteeman commented Oct 27, 2019

Uh oh!

alikon commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

alikon commented Oct 27, 2019

Uh oh!

infograf768 commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

infograf768 commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alikon commented Oct 27, 2019

Uh oh!

richard67 commented Oct 27, 2019

Uh oh!

infograf768 commented Oct 27, 2019

Uh oh!

anibalsanchez commented Oct 27, 2019

Uh oh!

infograf768 commented Oct 28, 2019

Uh oh!

richard67 commented Oct 28, 2019

Uh oh!

richard67 commented Nov 12, 2019

Uh oh!

alikon commented Nov 12, 2019

Uh oh!

wilsonge commented Nov 12, 2019

Uh oh!

infograf768 commented Oct 27, 2019 •

edited

Loading

alikon commented Oct 27, 2019 •

edited

Loading

richard67 commented Oct 27, 2019 •

edited

Loading