$rbac->Users->AllRoles() throws error if no parameter is passed

[jburns131]

The comments state,

* @param integer $UserID
*           optional

yet a php error is thrown if no parameters are supplied.

Proposed fix:

Old

function AllRoles($UserID)

New

function AllRoles($UserID = null)

[abiusx]

ok
nice test
having 100% code coverage will fix these.

---

Notice: This message is digitally signed, its source and integrity are verifiable.
If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Mordad 31, 1392, at 4:10 AM, Jesse Burns notifications@github.com wrote:

The comments state,

• @param integer $UserID

•       optional
  

  yet a php error is thrown if no parameters are supplied.

Proposed fix:

Old

function AllRoles($UserID)
New

function AllRoles($UserID = null)
—
Reply to this email directly or view it on GitHub.

[jburns131]

As per our discussion in Issue #2, I propose that we make $UserID mandatory and create/throw an Exception if $UserID is null.

[abiusx]

i belive allRoles should return all roles (not those of the user), check again. if its what you think the soution is right

---

Notice: This message is digitally signed, its source and integrity are verifiable.
If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Shahrivar 1, 1392, at 8:13 AM, Jesse Burns notifications@github.com wrote:

As per our discussion in Issue #2, I propose that we make $UserID mandatory and create/throw an Exception if $UserID is null.

—
Reply to this email directly or view it on GitHub.

[jburns131]

This method's intent/purpose is to return roles assigned to a user.

I have one concern regarding returning an object for this method. In either case we will be returning data for multiple items (every roll assigned to a user). I think we should probably stick with the array, because if we return an object it will still contain multiple items and in essence still be a multidimensional array.

I think if we were to return a single entity then an object would be ideal, but I'm not too sure about multiple entities. That sounds like the job and purpose of an array.

Let me know what you think.

P.S. Your response sparked an idea. I've created Issue #9 as a proposal.

[abiusx]

hmm
thats a sound point. lets return arrays, they are easier to convert into objects, but one should definitely know what indexes will be in the array.
-A

---

Notice: This message is digitally signed, its source and integrity are verifiable.
If you mail client does not support S/MIME verification, it will display a file (smime.p7s), which includes the X.509 certificate and the signature body. Read more at Certified E-Mail with Comodo and Thunderbird in AbiusX.com

On Shahrivar 1, 1392, at 1:55 PM, Jesse Burns notifications@github.com wrote:

This method's intent/purpose is to return roles assigned to a user.

I have one concern regarding returning an object for this method. In either case we will be returning data for multiple items (every roll assigned to a user). I think we should probably stick with the array, because if we return an object it will still contain multiple items and in essence still be a multidimensional array.

I think if we were to return a single entity then an object would be ideal, but I'm not too sure about multiple entities. That sounds like the job and purpose of an array.

Let me know what you think.

P.S. Your response sparked an idea. I've created Issue #9 as a proposal.

—
Reply to this email directly or view it on GitHub.

[jburns131]

Hey, I just realized that I responded to the comment about array's vs objects in the wrong Issue. The issue we should have had this conversation in is Issue #7.

I've added these comments to Issue #7 for future reference.

[jburns131]

I've addressed this Issue in commit aebafee

Please review and close Issue if everything looks good to you.