Skip to content

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

Merged
infograf768 merged 1 commit intoinfograf768:4.0_batch_copy_articlefrom
richard67:patch-1
Oct 28, 2019
Merged

[4.0] Use featured up and down times from copied article, if set#54
infograf768 merged 1 commit intoinfograf768:4.0_batch_copy_articlefrom
richard67:patch-1

Conversation

@richard67
Copy link
Copy Markdown

PR for joomla#26835.

Just have tested here, seems to work.

@infograf768
Copy link
Copy Markdown
Owner

I am confused...
$table can't at the same time represent _content with the featured column and _content_frontpage with both featured_up and featured_down columns.

It looks like the query should fetch publish_up and publish_down

@richard67
Copy link
Copy Markdown
Author

@infograf768 Table contains those columns because it has them copied in the Content.php table class. I know, it is strange.

@richard67
Copy link
Copy Markdown
Author

Maybe @SharkyKZ could check if it should be done in a different way?

@richard67
Copy link
Copy Markdown
Author

Here is the strange function which does it:

https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Table/Content.php#L387.

@infograf768
Copy link
Copy Markdown
Owner

😬

@richard67
Copy link
Copy Markdown
Author

Agree, is all ... let me say friendly ... not optimal with the featured up and down ... at the beginning it looked like it could be a nulldate problem so I became involved ... and now I wish I was in holiday somewhere where is no internet.

@SharkyKZ
Copy link
Copy Markdown

As I said before, I'm not sure whether we should be loading properties from foreign tables into table object like this. To me this looks wrong. Maybe a maintainer should take a look at this and decide.

@richard67
Copy link
Copy Markdown
Author

@SharkyKZ To me it also looks wrong, fully agree with you, but when I tried to fix it here I failed, so I thoight there was something which I missed or did not unserstand, and so i gave it up and started to fix what I could see in PHP logs.

@richard67
Copy link
Copy Markdown
Author

And so this thing is a bit like a pain in ...ss.

@richard67
Copy link
Copy Markdown
Author

@SharkyKZ I think I slowly develop an idea how the featured up and down should be done in the right way. Not sure if I can already provide a fix today, but tomorrow and Tuesday I have time, too. Let me know if you want to do the same and will be faster. I'll have a break right now for a few hours.

@SharkyKZ
Copy link
Copy Markdown

You can do it if you want.

@infograf768 infograf768 merged commit 1dccadb into infograf768:4.0_batch_copy_article Oct 28, 2019
@infograf768
Copy link
Copy Markdown
Owner

Merged for now to go on solving batch copy.

@richard67 richard67 deleted the patch-1 branch October 28, 2019 09:04
@richard67
Copy link
Copy Markdown
Author

@SharkyKZ For now it works here, but I agree with you, the featured up and down times have to be handled in another way, models have to do that, collect data from different tables, not the content table class has to copy columns from other tables. This should be fixed with a future PR.

infograf768 pushed a commit that referenced this pull request May 31, 2020
* Remove condition from workflow component

* Load Workflow plugins on transition generation

* Remove condition from com_content
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.

3 participants