Cancel link on delete message page has an incorrect redirect
| Project: | Privatemsg |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
First up such a great improvement to the D5 version of PM. Awesome.
I have tested this to be very sure and this is the bug I have found:
When viewing any message and I want to delete parts of the message from any participants I am directed to a page (standard drupal style) to confirm I want to delete or cancel. The delete works fin deleting the section and redirecting me back to the inbox. I think it would be so much better to redirect the user back to the message, just an idea. Now the bug I have found is if you choose to cancel you are redirected to a page not found. I believe this is becos it is taking the thread message as the actual message in the url, so when clicking cancel of course this url does not exist.
Let me know if this is unclear but I believe this needs some tiny tweak to fix.
Cheers
Glynster
#1
confirmed. cancel link redirects to $mid rather than $thread_id. it needs to redirect to $thread_id of the $mid which is being deleted.
#2
Work in progress patch.
Current changes include:
1) When deletion is confirmed the user is redirected back to the thread view
2) when cancel link is clicked the user is redirected back to the thread view
problem:
When last message in thread is deleted, the user is still redirected to the thread view which gives him a 404. We need to check if thread is non-empty before redirecting to the thread view, otherwise to inbox with appropriate DSM status message of why they landed there instead of thread view.
I'll implement the above in the following days if no one picks up on it.
#3
Great job and so quick on the reply.
Again such an awesome module, keep up the great work.
Also your latest apply to the tags so much better I think, very logical and less work for the user.
Nice one.
#4
The attached patch should work. It is a bit an overkill to use privatemsg_thread_load() just to count the number of remaining messages, but performance should not be an issue in such a place I think. The alternative would be to introduce a new helper function, with a new query definition.
#5
And now with patch...
#6
What about embedding the thread_id into the url: "messages/delete/%thread/%message" ?
or you could just add an extra bit to the url: "messages/delete/%message&thread=%" ?
good ideas or bad?
I am just thinking that with the above loading of a thread, it is technically possible to have a message in multiple threads, so you may not get a unique/correct value.
#7
Thanks for the update guys.
So awesome to be using a module that is well maintained.
Cheers
G
#8
Updated, the destination param is now set when generating the url, this does eleminate the need to call the message or thread inside the actual delete functions. Since privatemsg_thread_load() is statically cached, this shouldn't be a performance issue.
#9
Well, the idea was nice, but it does not work. The reason is that cancel should always return to the view message screen....
Changes:
- I'm now using messages/delete/thread_id/mid with named wildcards to make the code simpler
- Fixed a notice that delete_options is undefined
- Fixed a bug when pm_block_user is not enabled, since thread_id was only loaded if that module was enabled
- Wrote a few tests.
#10
Updated to make this work after the privatemsg_message_load_multiple() change.
#11
Fixed in 6.x-1.x-dev and 7.x-1.x-dev.
#12
In my 6.x-1.0-rc4 doesn't work, any ideas??
#13
read again the message above, in which version it is fixed... then try right version.
#14
well but i need an email notification which appeared in 6.x-1.0-rc3, and in general rc4 is nicer, dont understand why to keep this bug in newer versions, maybe missing sth
#15
1.x-dev is newer than rc3 and is generally just as stable - but it can have more bugs as there is work ongoing to add features and other changes too.
Your options are to move onto the 1.x-dev branch and risk some instability, or wait for the next release, which should not be too long away.
#16
ah ok, thank you very much for your help
#17
Automatically closed -- issue fixed for 2 weeks with no activity.