code42 / py42

MIT License
19 stars 15 forks source link

[Bug] get_all_matter_custodians doesn't like not specifying holdMembershipUid, holdUid, userUid, or userSearch #228

Open phirst opened 3 years ago

phirst commented 3 years ago

get_all_matter_custodians Doesn't Like Just the Defaults

The py42 documentation ( https://py42docs.code42.com/en/stable/methoddocs/legalhold.html ) for get_all_matter_custodians states that all of the parameters ( legal_hold_uid, user_uid, user, and active ) are optional. Not including the uid or user parameter generates the following error:

Fatal Error : Failure in HTTP call 400 Client Error: Bad Request for url: https://console.us.code42.com/api/LegalHoldMembership?activeState=ACTIVE&pgNum=1&pgSize=1000
Exiting...  Please check the log file for details.

Executing the same call ( https://console.us.code42.com/api/LegalHoldMembership?activeState=ACTIVE&pgNum=1&pgSize=1000 ) from a browser results in:

[
{
name: "SYSTEM",
description: "Invalid request: SYSTEM com.code42.core.BuilderException: At least one criteria must be specified; holdMembershipUid, holdUid, userUid, or userSearch"
}
]

Steps to Reproduce

  1. Initialize the SDK
  2. Make the request: legalhold_users = sdk.legalhold.get_all_matter_custodians(active=True)

Expected Behavior

Return all of the legal hold users.

Actual Behavior

Fatal Error : Failure in HTTP call 400 Client Error: Bad Request for url: https://console.us.code42.com/api/LegalHoldMembership?activeState=ACTIVE&pgNum=1&pgSize=1000 Exiting... Please check the log file for details.

Basic Information

alanag13 commented 3 years ago

thanks for reporting this @phirst ! I think this is a documentation issue more than anything else -- the reason both those parameters are optional is because neither one of them is required -- you have to do at least one or the other, but I don't think we should change this to make it so that those are both required parameters. I agree that the documentation is not clear about this though, so we should adjust that.

alanag13 commented 3 years ago

we of course cant just remove this function, but I think in a future version of py42 (2.0) we could remove uid from this version of the function and add an additional function that is something like get_all_matter_custodians_by_id() that requires the id as a parameter.

phirst commented 3 years ago

It would be nice to have the ability to get all the custodians in a single shot rather than have to get all the matters then loop through all the matters (even though that's what the SDK would likely be doing anyway).

I'll adjust my code to loop through the matters...

alanag13 commented 3 years ago

A convenience method to get all custodians is definitely doable too, thanks for that feedback!