OnClick Javascript attached to Custom button not working correctly

Question

I have a custom button in a layout created for a custom object. Upon clicking, if some conditions are fulfilled, I would like to execute some code and otherwise I want to display a popup. I use the OnClick functionality provided by Salesforce and I inserted the following code:

{!REQUIRESCRIPT("/soap/ajax/26.0/connection.js")} 
{!REQUIRESCRIPT("/soap/ajax/29.0/apex.js")} 

if(!("{!Invoice__c.RecordType}" != "2 – Invoice" || "{!Invoice__c.Invoice_Status__c}" != "Credited"))
{
   var newInvoiceId = sforce.apex.execute("InvoiceCloning","cloneInvoice", {invoiceId:"{!Invoice__c.Id}"});
   window.location.href = "/"+newInvoiceId;
}
else
{
   alert("Cannot create credit note");
}

The conditions look pretty straightforward and also the if - else statement is nothing transcendental. However just the code inside the else is executed. I tried to debug with Chrome and I found out that the values are correctly populated and I do not understand why the expression in the if statement evaluates to false:

<input value="Create Credit" class="btn" name="create_credit" title="Create Credit" type="button" onclick="if (window.invokeOnClickJS_00b8A000000Dhcm) window.invokeOnClickJS_00b8A000000Dhcm(this); else if (parent.window.invokeOnClickJS_00b8A000000Dhcm) parent.window.invokeOnClickJS_00b8A000000Dhcm(this); return false">

Code that exectues Onclick:

window.invokeOnClickJS_00b8A000000Dhcm = function(element) {
  function getRecordIds(keyPrefix) { return Scontrol.prototype.getSelectedRecordIdsFromForm(element.form, keyPrefix); }

  element.invokeAction = function() { 
  var scriptLibrary = [];
scriptLibrary.push("/soap/ajax/26.0/connection.js");
scriptLibrary.push("/soap/ajax/29.0/apex.js");
Util.loadScripts(scriptLibrary, 0, function() {
    if (typeof(sforce) != 'undefined' && typeof(sforce.connection) != 'undefined') { sforce.connection.sessionId = '00D8A0000000POh!AQkAQGNGOdzEJLjjmvJrMdCB62tq9Z_jczpq6u4UKrCGycXIZfTjxYxFzk7fBUaMjB8ZVFDK.jOuKQ6UG6JGQyC_5chaaC_.'; }
    try { Util.stripCustomFunctionFromObjectPrototype(Array);eval(' \r\n \r\n\r\nif((\"2 - Invoice\" == \"2 – Invoice\" && \"Credited\" == \"Credited\"))\r\n{\r\n\r\nvar newInvoiceId = sforce.apex.execute(\"InvoiceCloning\",\"cloneInvoice\", {invoiceId:\"a7q33000000L7zl\"});\r\nwindow.location.href = \"/\"+newInvoiceId;\r\n\r\n}\r\n\r\nelse\r\n{\r\nalert(\"Cannot create credit note\" + \"2 - Invoice\" + \"Credited\");\r\n}') } 

catch (e) { alert('A problem with the OnClick JavaScript for this button or link was encountered:\n\n' + (e.message ? e.message : e)) }

finally { Util.restoreCustomFunctionsToObjectPrototype(Array);}
    });
      };
    element.invokeAction();

    }

What am I doind wrong? Thanks

Answer 1

As an addendum to Ilya's good answer, I thing a good way of getting clarity in the if statement is to remove some of the binding logic and evaluate it prior.

What I'd normally do if my if statement was getting difficult to understand would be to either use a function to evaluate the expression - ie:

if( isCreditedInvoice())

Or (more likely in this case):

var isInvoice = "{!Animal__c.RecordType}" == "2 – Invoice" ? true : false;
var isCredited = "{!Animal__c.Invoice_Status__c}" == "Credited" ? true : false;

Then you can write the lovely clear statement:

if (isInvoice && isCredited){
    //do stuff
}

And of course, you could try this option too:

var recordType = "{!Animal__c.RecordType}";
var invoiceStatus = "{!Animal__c.Invoice_Status__c}";

You statement will look like this:

if (recordType == "2 - Invoice" && invoiceStatus == "Credited"){
    //do stuff
}

Anyway, all this is saying is that if you abstract away some of the nuts and bolts, you can make your job easier.

Answer 2

Your condition:

if(!("{!Animal__c.RecordType}" != "2 – Invoice" || "{!Animal__c.Invoice_Status__c}" != "Credited")) 

will be executed only if condition inside parenthesis is false, meaning that both comparison should be false. e.g. RecordType == "2- invoice" and status == "Credited"

This can be replaced with:

if(("{!Animal__c.RecordType}" = "2 – Invoice" && "{!Animal__c.Invoice_Status__c}" = "Credited"))

Or you migth be confused with first explanation mark, if you remove it, condition fire if:

if RecordType != "2- invoice" Or status != "Credited"