Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stripping linebreaks when removing attachment. #9764

Merged
merged 11 commits into from
Nov 14, 2024
Merged

Conversation

Pinkeltje
Copy link
Contributor

Stripping linebreaks when removing attachment.
@rich20
Copy link
Member

rich20 commented Nov 5, 2024

  • It works when you use the ‘Remove File’ button
  • The problem still exists with the ‘Remove all’ button

@Pinkeltje
Copy link
Contributor Author

Will check remove all. Didn't test it.

@Pinkeltje
Copy link
Contributor Author

Pinkeltje commented Nov 5, 2024

Not smart enough to solve this. Line breaks are not stripped anymore, and when clicking Remove all it looks like all attachments are removed,
2024-11-05 Screenshot 675
but then when editing message again, only one attachment is removed from database.
2024-11-05 Screenshot 676
Furthermore there seems te be a layout issue
2024-11-05 Screenshot 674

@rich20 rich20 added this to the 6.3 milestone Nov 5, 2024
@rich20
Copy link
Member

rich20 commented Nov 5, 2024

Not smart enough to solve this. Line breaks are not stripped anymore, and when clicking Remove all it looks like all attachments are removed,

Yes, that is correct.

but then when editing message again, only one attachment is removed from database.

It must have something to do with this fix. If there are several images in the post, they are removed from the post with ‘Remove All’, but only the first image is removed from the database and the attachment folder.
If I test it without this fix, all images including database entries are removed correctly.

Furthermore there seems te be a layout issue

This error also occurs without this correction. If you use the ‘Insert All’ button, the ‘Set as Private’ button also changes to ‘In Message’.

Summary:

  • Remove File button- Line breaks remain in place
  • Remove All button - Line breaks remain in place
  • Remove All button - Only the first file is removed from the folder and the database
  • Insert All button - Set as Private changes to In Message (see image)
    .
    2024-11-05 Screenshot 674

@Pinkeltje
Copy link
Contributor Author

Pinkeltje commented Nov 6, 2024

Insert All button - Set as Private changes to In Message (see image)

@rich20 Can you pls test with clean K6.3.6. I've just installed a clean K6.3.6 and buttons are wrong as well there.

@rich20
Copy link
Member

rich20 commented Nov 6, 2024

@Pinkeltje This error also occurs with a fresh installation. That's why I wrote that:

This error also occurs without this correction

With this, I meant to say that this bug already existed before I tested your fix.

Removed Insert private attachment as this doesn't work. Inserted private attachments are still visible for everyone.
Buttons are now ok.
@Pinkeltje
Copy link
Contributor Author

Removed insert private button as inserted private images were still visible to all users (and public). Button layout is now ok.

@rich20
Copy link
Member

rich20 commented Nov 8, 2024

Removed insert private button as inserted private images were still visible to all users (and public). Button layout is now ok.

Private attachments may not be inserted into the message.

Now the ‘Set as Private’ button is completely missing and therefore it is not possible to set individual attachments as private. (The fact that the recipient cannot see private posts or private attachments is another bug). #9559

The problem in general is much bigger than it seems.

  1. if an attachment is set as private, the ‘Insert’ button should disappear so that a private attachment cannot be inserted into the message. At the same time, the text and colour of the ‘Set as Private’ button should change to indicate that the attachment is private.
  2. if an attachment has been inserted into the message, the ‘Set as Private’ button should disappear, as an inserted attachment can no longer be set as private.
  3. the same should happen when the ‘Insert All’ and ‘Set all attachments as private’ buttons are clicked. Either all should be displayed as ‘In Message’ or as ‘Is Private’.

Example image: One image was inserted as private and one in the message.
.
image14570

@Pinkeltje
Copy link
Contributor Author

I see it. Didn't test that. The private issue is a real problem.
Maybe ditch it completely for the time being and take the time to find a working solution for both problems? I've disabled it on our forum.
Still struggling with remove all BTW.

Remove all working I think
@Pinkeltje
Copy link
Contributor Author

Seems like remove-all works now with latest commit.

@rich20
Copy link
Member

rich20 commented Nov 8, 2024

Remove all works now. But individual attachments can now only be removed in edit mode.

Test:

  1. add two or more attachments
  2. remove one again

This only works if you save the post first and then edit it

buttons display fixed and remove individual attachment now possible
@Pinkeltje
Copy link
Contributor Author

@rich20 Can you test again please?

@rich20
Copy link
Member

rich20 commented Nov 11, 2024

  • Insert All - Success
  • Remove All - only works if at least one attachment has been added into the message beforehand. If this is not the case, the button changes to ‘Set all attachments as private’ when clicked and attachments can then no longer be removed.
  • Set all attachments as private works, but you cannot recognise it because no ‘Is private’ is displayed. You can only check it with a second user account.
  • Set as Private is the same. It works, but you cannot recognise it because no ‘Is private’ is displayed. You can only check it with a second user account.

Added buttons Is Private and all attachments private
Requires adding language constants COM_KUNENA_ATTACHMENT_IS_PRIVATE and COM_KUNENA_ALL_ATTACHMENTS_ARE_PRIVATE added to transifex
@Pinkeltje
Copy link
Contributor Author

Should be better now but requires adding language constants COM_KUNENA_ATTACHMENT_IS_PRIVATE and COM_KUNENA_ALL_ATTACHMENTS_ARE_PRIVATE added to transifex

@rich20
Copy link
Member

rich20 commented Nov 12, 2024

Should be better now but requires adding language constants COM_KUNENA_ATTACHMENT_IS_PRIVATE and COM_KUNENA_ALL_ATTACHMENTS_ARE_PRIVATE added to transifex

I then write a separate ticket for the language strings.

  • Insert All - Success
  • Remove All - Success
  • Set all attachments as private - The Insert and Insert All buttons remain visible
  • Insert (single image) - The button Set as Private remain visible
  • Set as Private (single image) - The button Insert remain visible

@Pinkeltje
Copy link
Contributor Author

Do you mean one shouldn't be able to insert private attachments?

@rich20
Copy link
Member

rich20 commented Nov 12, 2024

An attachment that has been inserted directly into the message can no longer be Set as Private. Or it would have to be removed from the message, as attachments that have been inserted into the post are no longer private. Private attachments can only be uploaded and set as private.
However, if an attachment has been inserted into the message and the button to set it as private is still present, this is irritating.
It is also irritating if an attachment marked as private can still be inserted into the message because the Insert button is still available for this attachment.
To summarise:
If an attachment has been set as Private, the Insert button for this attachment should disappear.
If an attachment is included in the message, the Set as Private button for this attachment should disappear.
In principle, this should prevent an attachment from being is set as private and public at the same time.

@rich20
Copy link
Member

rich20 commented Nov 12, 2024

As for the new language strings:
We already have similar strings for these options here. You can, if you want, create a pull request for these new required strings, then they will be automatically added to Transifex after the merge.
The right place for this would be en-GB.com_kunena.ini starting at line 54

COM_KUNENA_EDITOR_INSERT = "Insert"

@Pinkeltje
Copy link
Contributor Author

An attachment that has been inserted directly into the message can no longer be Set as Private. Or it would have to .....
Agree. Tried already to change code but is complicated.

hide insert when private is clicked and hide private when insert is clicked for individual attachment and all attachments
changed language constants to existing language constants for private buttons
All buttons should now be hidden as expected.
@Pinkeltje
Copy link
Contributor Author

@rich20 I think it works ok now with latest commit. Found already existing language strings in language file (bit confusing as insert has "PRIVATE" in constant and Is private has "SECURED" in it.
Might have still missed something in my tests but as far as I can see it works.

@rich20
Copy link
Member

rich20 commented Nov 13, 2024

  • Yes, now works as it should

@rich20
Copy link
Member

rich20 commented Nov 13, 2024

Edit messages with attachments
hm... I found one more thing and that is when editing. When the post is saved and then edited, the attachments information should be preserved. When editing, all the options for the existing attachments will reappear, so you can private attachments insert into the post, or vice versa, you can also set attachments that have already been added into messages to private.

@rich20
Copy link
Member

rich20 commented Nov 14, 2024

Now it's almost perfect. There is just one more problem. If you remove a single attachment was not inserted in the post, the text is also removed from the editor. This happens both when creating a reply and when editing a post.

Test:
Enter a text in the editor
Add an attachment
Remove the attachment again (or set it as private first and then remove it).
This error does not occur with Remove All.

@rich20
Copy link
Member

rich20 commented Nov 14, 2024

Thanks, everything is perfect now. I found another bug, but I don't know if it has anything to do with the upload.main.js.
If users scroll down when replying, they can see private attachments in the Topic History, even if they are not authorised to do so.
Do we need a separate bug ticket for this?

@Pinkeltje
Copy link
Contributor Author

I think this is another bug as the changes only affect the buttons and actions.
There is another bug too. Even when attachments are removed the attachment icon stays visible in recent topics.
I suppose there is no relation between attachments table and attachment field in topic table.

@rich20
Copy link
Member

rich20 commented Nov 14, 2024

I also think that it has nothing to do with the buttons and will write an extra ticket.

  • Bug Stripping linebreaks... - Solved
  • Buttons all ok now

@xillibit xillibit merged commit a540717 into Kunena:K6.3 Nov 14, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants