Skip to content

[4.0] Batch copy workflow association, too#55

Merged
infograf768 merged 1 commit intoinfograf768:4.0_batch_copy_articlefrom
richard67:4.0-dev-fix-articles-batch-copy-workflow-assocs
Nov 13, 2019
Merged

[4.0] Batch copy workflow association, too#55
infograf768 merged 1 commit intoinfograf768:4.0_batch_copy_articlefrom
richard67:4.0-dev-fix-articles-batch-copy-workflow-assocs

Conversation

@richard67
Copy link
Copy Markdown

PR for joomla#26835.

Just have tested here, seems to work.

If you want, merge this and change the scope of your PR to handle the general batch copy problem, too.

@infograf768
Copy link
Copy Markdown
Owner

not at desktop right now, will look later.

@richard67
Copy link
Copy Markdown
Author

Hmm, there seems to be a PR already doing it, joomla#24519 . Is RTC but seems to be more complicated than my way. But maybe it's the more correct way? Anyway. Is RTC => someone merge.

@richard67
Copy link
Copy Markdown
Author

Closing because there is PR joomla#24519 already solving that.

@richard67 richard67 closed this Oct 28, 2019
@alikon
Copy link
Copy Markdown

alikon commented Oct 28, 2019

why closing this with only 3 line of code using API, instead of joomla#24519 which is not using API ?

@richard67 richard67 reopened this Oct 28, 2019
@richard67
Copy link
Copy Markdown
Author

Reopened. But I am not sure if what I do here is correct. Haven't checked the other PR in details but maybe the other starts workflow for copied articles from beginning, while my patch just copies the workflow association with the same workflow state as the copy source.

@wilsonge As soon as you find time, could you check what is right, this here or joomla#24519?

@richard67
Copy link
Copy Markdown
Author

I've just checked what joomla#24519 does, and left a comment there: joomla#24519 (comment).

@alikon @infograf768 Please check and comment there.

I don't want to discourage some new contributor by replacing his PR, so I thought I should give him a chance to change that himself.

@infograf768
Copy link
Copy Markdown
Owner

@richard67
The more I think of it, the more confused I get.

We have 2 cases:

  1. we copy to the same category.
    In that case keeping the existing workflow is fine.

  2. We copy to another category
    Should'nt the workflow be the one used for that category?

@richard67
Copy link
Copy Markdown
Author

Yes, maybe that’ the reason why that other PR is so complicated? I have to check tonight after work, German time.

@richard67
Copy link
Copy Markdown
Author

@infograf768 I've just checked: It seems that other PR makes a few checks and resets some flags like e.g. published, but the workflow association is handled in the same way as I do, i.e. just copy with same workflow ID as the old article has. But I am not sure if I understand all right. Let's see if someone answers my comment there.

@wilsonge
Copy link
Copy Markdown

IF your PR works it looks far more sensible to me

@richard67
Copy link
Copy Markdown
Author

@wilsonge It works, and as far as I can see most of the code added PR joomla#24519 is identical with the code in the base class, only insert of a new workflow association is added to that, and so it seems to do the same as my PR here does. But there was so much review done on that PR by people of who I think that they have much more knowledge on that than I have ... why hasn't someone found the easier solution before me? This is what makes me worry that I might be missing something. And I don't want to discourage a new contributor by throwing away his PR.

@wilsonge
Copy link
Copy Markdown

wilsonge commented Nov 3, 2019

OK in which case this is definitely the fix

@infograf768 infograf768 merged commit 6c6ef3d into infograf768:4.0_batch_copy_article Nov 13, 2019
@infograf768
Copy link
Copy Markdown
Owner

tks

@richard67
Copy link
Copy Markdown
Author

Thanks too.

@richard67 richard67 deleted the 4.0-dev-fix-articles-batch-copy-workflow-assocs branch November 13, 2019 10:40
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

* Cleanup content from workflow
infograf768 pushed a commit that referenced this pull request Dec 17, 2020
Sample Data - Banner and Newsflash
- Add some text and a button in the banner overlay the banner area.
- Add some intro images to articles
- Make the MenuItem "Blog" a Link to a Blog in mansonry layout
- Some modules which are not < nav > are tagged as < aside > (a11y) see chart in comment #55 (comment)
- A newflash module has been added. There are three horizontal boxes in position top-a.
- These articles have an intro-image, at the moment it is the banner image.
- Blog featurd (Template) is in a two-columns layout with one leading article, using modifier classes for blog-layout..
- Newslash and banner are only on the homepage.
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.

4 participants