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

dev/core#1784 fix regression on restore contacts button not working. #17418

Merged
merged 1 commit into from
May 31, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

This should fix https://lab.civicrm.org/dev/core/-/issues/1784

I think the message from deleting multiple contacts is a bit wonky, but no worse

Before

Contact not restored

After

Contact restored

Technical Details

@demeritcowboy

Comments

This should fix https://lab.civicrm.org/dev/core/-/issues/1784

I think the message from deleting multiple contacts is a bit wonky, but no worse
@civibot
Copy link

civibot bot commented May 28, 2020

(Standard links)

@civibot civibot bot added the 5.26 label May 28, 2020
$session = CRM_Core_Session::singleton();
$currentUserId = $session->get('userID');

$context = CRM_Utils_Request::retrieve('context', 'Alphanumeric', $this, FALSE, 'basic');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All moved to $this->setRedirection()

@@ -228,15 +212,7 @@ public function postProcess() {
}
if ($deleted) {
$title = ts('Deleted');
if ($this->_restore) {
$title = ts('Restored');
$status = ts('%1 has been restored from the trash.', [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems wrong for multiple contacts but should be no worse for this PR (it might be an empty name rather than a random one but not sure that is worse)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works because of the 'plural' and 'count' params to ts(). You don't see it that much - came from Drupal I think.

@demeritcowboy
Copy link
Contributor

Thanks! Will take a look.

@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] PASS
    • [r-doc] PASS
    • [r-run] PASS
      • (Preexisting weirdness) If you delete from search results, then it takes you back to the search results screen with the contact(s) still in the list, even though they have actually been deleted. And in fact if you go somewhere else, start the search again from scratch and search for the same thing they still show in the list.
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
    • [r-maint] Undecided
      • Given that this slipped thru before, it's not a covered area. I'll see if I can make one up.
    • [r-test] PASS

@eileenmcnaughton eileenmcnaughton merged commit eaba3c3 into civicrm:5.26 May 31, 2020
@eileenmcnaughton eileenmcnaughton deleted the trash branch May 31, 2020 06:29
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy thanks for the review - no new concerns + fixes regression from above so I have merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants