Approved bookings should not be editable or deletable
Brought to you by:
jberanek
Menu
▾
▴
#1824 Approved bookings should not be editable or deletable
Hi,
I've seen the ticket at: https://sourceforge.net/p/mrbs/support-requests/1563/
In that, you answer only to the part regarding past bookings. However it seems to me that there is no way to stop the booker from editing or deleting an already approved booking. Is this correct?
I find it necessary, do you have any suggestions?
I'm using 1.8.0
Thank you for mrbs! Paolo
It's not possible unless you modify the MRBS code.
OK. I thought so.
As I see by now it would not be very complicated because it is a matter of adding a configuration variable and adding one more policy stop.
The policy is checked via an ajax call from edit_entry.js.php (checkConflicts()) that as for now I don't think needs any changes.
The changes are all in mrbs_sql.inc (mrbsCheckPolicy()) that is called by edit_entry_handler.php.
It should be enough to add a single check for delete operations if, as I assume, no edit operation is performed when the initial delete operation is forbidden by the policy violation.
The rest is just to add a description of the policy violation to the lang files.
I would like to give it a try if you think the change is not much diffferent from what I describe here and if you think, like me, that it could be a useful addition.
The last hurdle for me would be that I use git and not mercurial, so I would need directions for how to send you the patch.
/paolo
Yes, I think that's all you need do. Just post the diffs or new files here. Thanks.
Hi again,
Here are the changed files.
I have added messages to the English lang file but also Danish and Italian (the two languages I know very well). In the Italian file I have also fixed two occurrences of a non utf-8 charachters.
In view_entry.php I felt like completely removing the edit and delte buttons when those operations were conflicting, but I guessed that it did not follow mrbs general inclination, so I opted for passing a specific text to the javascript alert that explains the reason why those bookings won't be deleted.
I hope it's OK. Just let me know if I need to change something.
I have one more whish of new configuration that I would like to add but I will write it in a feature request.
Thanks for your attention
Well, just noticed that I left an unused:
global $approved_bookings_are_blocked;
at the top of view_entry.php
will you remove it or shell I send a new set fo files?
I've just noticed that mrbs has a PHP escape_js function, so the text that I popoup in the alert doesn't need to be escaped in the language string.
Here is a new set of files that uses escape_js and fixes the dangling
global $approved_bookings_are_blocked; I wrote above.
I hope this is the last time :-) however here is a new version of the lang.it file, where the new sentences are more in line with the existing ones.
Furthermore, the original lang.it file did also escape the single quote (used as an apostrophe ) so in this version the single quote is not escaped even on
$vocab["confirmdel"] = "Sei sicuro che vuoi cancellare l'elemento?";
I have checked that no other languge is using a single quote in $vocab["confirmdel"]
Thanks for this. I'll try and have a look next wek.
I have now (69f48e) committed the new policy to the default branch. I have made a couple of changes to your code:
$approved_bookings_cannot_be_changed.I'll look at the view_entry changes later, though I'm inclined to follow your original suggestion of not showing buttons if they can't be used.
Great thank you!
Reg. view_entry and removing the buttons: one issue could be that the user might get confused because they do not necessarily know that an already approved booking cannot be changed. The old method let the user waste time by clicking back and forth, but they get an explanation in return.
However, it would be OK to me either way.
Maybe another solution would be to keep the button, but disable it, and add a tooltip explaining the reason.
I have corrected the lang.it translations in af4df5.
yes, that would definitely be the best solution
Last edit: Campbell Morrison 2019-09-07
I have now implemented this in 04ba39.
The Edit and Delete buttons will now be disabled if the entry can't be deleted due to a booking policy. However if the individual entry can be deleted, but the series can't - because another entry in the series cannot be deleted - then the series buttons will not be disabled. That's a bit more complicated to implement and will have to wait for another day. But for the moment the changes should be a big improvement on the previous behaviour of MRBS.
It's very nice, thank you.
However a new bug has been introduced due to the new italian translation. I explain:
The new text corrected some non utf8 chars, but it also removed escaping the single quote directly in the language file.
This was done because of what I wrote in one message above:
I've just noticed that mrbs has a PHP escape_js function, so the text that I popoup in the alert doesn't need to be escaped in the language string..So the alert message inside view_entry.php must be escaped using this function with:
$button_attributes = array('onclick' => "return confirm('" . escape_js(get_vocab("confirmdel")) . "');");otherwise the single quote in the language string gives an exception in javascript and that knocks out the alert in the cases where policy does not forbid deletion but a confirmation is required.No other language uses single quotes in the language strings and have escaped it in the language file. I believe the right thing to do is to escape the string in the code and not to escape the single quote in the string itself.
Sorry, I missed that. Now fixed in 899f41.