CRM-21478 Using contribution status id instead of label in if statement

[samuelsov]

Overview

On localized site with payment status translated, the dashboard fail to display some useful information.

Before

• When payment is pending, it's not possible to see the "Pay Now" button
• If taxes and receipting is enabled, when payment is canceled or refund, "Print Invoice" is displayed instead of "Print Invoice and Credit Note"

After

Comments

Really minor changes that only impact this screen.

---

• CRM-21478: Pay Now on user dashboard only works in english

[mattwire]

I think the normal syntax would be neq instead of !=

[eileenmcnaughton]

Hmm - it always used to be that smarty writers used that format - then we discovered 'normal php' format worked & now it's pretty mixed. My preference is to migrate to this format on the basis it's more familiar to php / js writers and the smarty format is not really familiar to anyone

[mattwire]

Grepping *.tpl for contribution_status also found: https://github.com/civicrm/civicrm-core/blob/master/templates/CRM/Contribute/Form/ContributionView.tpl#L58

Can you update that too? And check if there is anywhere else.

[mattwire]

eq instead of ==

[eileenmcnaughton]

@samuelsov this isn't the right fix - we are hard-coding statuses. I think the right fix is to ensure that the contribution_status.name field is assigned to the template & we check against that. My understanding is that only the label field is translated. If I'm wrong about that we have to resort to assigning $refundStatusId & $cancelledStatusID to the template.

[mattwire]

@eileenmcnaughton @samuelsov I was going to make the same comment about hardcoding status IDs, but then I grepped and saw that there were quite a few "contribution_status_id" hardcoded and wasn't sure if it was in scope for this PR.
It kind of makes sense to me to merge this PR and then open another PR to replace hardcoded "contribution_status_id" with "contribution_status.name" - as we'll need to check if we've captured all the forms that assign parameters to the relevant templates (that might be easy, but it might not).

[eileenmcnaughton]

There is some risk if we merge this we fix it for some & break if for others. I know of one site off the top of my head that has a number other then 7 for refunded. I don't think we need to increase scope to fix all the hard-coded places though - although ideally we would at least add TODO comments into those locations

[samuelsov]

@eileenmcnaughton Ok, i though IDs were the same on any CiviCRM instance but name is much better of course if it's available in this context.

@mattwire ok for templates/CRM/Contribute/Form/ContributionView.tpl

[samuelsov]

@eileenmcnaughton @mattwire ok, i have fixed the id vs name problem in the user dashboard but i'm not so keen on expanding the scope of this issue as i find more and more problems as i dig. I'd rather document what i see in a new issue and commit this one.

[samuelsov]

I have opened a new issue for the remaining : https://issues.civicrm.org/jira/browse/CRM-21496
@mattwire If you see anything else please add a comment to the issue

[mattwire]

@samuelsov I haven't tested the patch but assuming contribution_status_name is set then it looks correct to me. And agree with raising a separate issue for the (similar) issues elsewhere.

[eileenmcnaughton]

I just tested on local & this does not regress on a site in English so the name value is working. Merging