Add plugin attachmentsid #81
Conversation
{attachmentsid id=xx,yy}
where xx,yy = id of the attachment
correct copyright
do not use require_once
inherit from PlgAttachmentsFramework
|
I think that we don't really need another plugin for this extension @JLTRY but we should incorporate the id= functionality in the attachments framework plugin. I have a partial implementation that seems to work but not correctly right now. I believe that I will have a draft by tomorrow that I will add to this PR if you don't mind. However I believe that for the functionality to be complete we should add another button in the editor that would allow you to select the attachment from a list as it would be difficult for most people to know the id of the attachment. |
|
Agree with you for keeping same plugin. To be noticed that mine is working. For button I have got a code that works with the choice of existing attachments, and I will make a pull request soon |
| } | ||
| else { | ||
| $query->where('a.parent_id='.(int)$parent_id); | ||
| if ($parent_id != '%') { |
There was a problem hiding this comment.
@JLTRY I cannot understand what you want to achieve here. I can see that you set parent_id to % earlier but since you check if parent isn't % why did you change the query from "a.parent_id=" to "a.parent_id like"? Did you want to add attachments from other articles maybe?
There was a problem hiding this comment.
yes sorry the query it should not be changed for a.parent_id. Yes if attachmentsid is not null it is of format xx,yy,zz. YEs I want to be able to add attachments from everywhere
| $model = $this->getModel('Attachments', 'Site'); | ||
| if ( !$model ) { | ||
| $errmsg = Text::_('ATTACH_ERROR_UNABLE_TO_FIND_MODEL') . ' (ERR 60)'; | ||
| throw new \Exception($errmsg, 500); |
There was a problem hiding this comment.
Why did you remove the exception here? Did you find a bug that something wasn't handled correctly?
There was a problem hiding this comment.
hello sorry yes I should put it back
| } | ||
|
|
||
| if ($attachmentsid) { | ||
| $parent_id = '?'; |
There was a problem hiding this comment.
Why do you change parent_id to ?
There was a problem hiding this comment.
the idea was to be able to select other attachments as current article
|
I have pushed the changes to make attachments framework plugin handle {attachments id=xx,yy,zz} style tokens. It doesn't handle attachments from other parents yet but can be modified to handle this use case if needed. Let's start small and expand later. I believe that most of the times the user would want to add all the files to the article and then add the needed file links in specific positions in the article. If we add attachments from other parents we might want to check if the parent is disabled/recycled/archived to allow adding/showing the attachment which will make things a bit more complicated. |
$attachmentid is not a list
|
I corrected some issue for attachmentsid |
|
@JLTRY the changes I pushed were for using just the original plugins with {attachments id=xxx,yyy} so that is why it broke the new plugin. Also it is not really safe to get text from the user and pass it as is to the query. That is why I used a regex that expects only digits and the comma character next to "id=". Try disabling attachmentsid plugin and use the token. It should work as is. |
|
Also I forgot a couple of log messages in the code that we should remove before merging. |
remove logs
|
Now it should work and I removed the log |
2 participants
{attachmentsid id=xx,yy}
where xx,yy.. are the id(s) of the attachment(s)
inspired by #3