Make 2 PATCHes at once

[mendykrauss]

How would I do this:

[ { "op": "replace", "path": "receivedStatus", "value": "PartiallyReceiveCloneRest" }, { "op": "replace", "path": "receivedQuantity", "value": 1 } ]

URL is: {{baseUrl}}/procurement/purchaseorders/:parentId/lineitems/:id

Worked in Postman but can't figure it out in the client.

Thanks!

[emichaud]

I was wondering the same thing but I don't think you can combine multiple field updates in a single update via ConnectPyse as it sits. Via the CW api, you would pass in a list of dict's that are formatted like your example.

looking at the update logic in the CWController, it builds that dict for you from the key,value you pass in but limits the list to one item. here is the function for example (from CWController.py)

def _update(self, item_id, key, value):
    # build PatchOperation dict
    patch_operation = [{
        'op': 'replace',
        'path': key,
        'value': value
    }]
    # call Patch method on API

...

If you tried to pass in a dict for the key and another for the value in the update function, you would end up with a dict inside of a dict rather than a list of dict's.

you could try subclassing CWController and override that method, but what is probably needed is a method that takes a list of dict with key/value and then builds the correctly formatted list.

I currently use the update heavily in one script and I just have to call it multiple times. It's simple, but it does bug me to make 5 or 6 separate updates when I could do it in one. Currently it works, so I'm living with it but ideally it could be optimized. When time permits, maybe I'll give it a try.

[mendykrauss]

Thanks for that.

The problem is, that the case that I gave is very specific and needs to be done in the same call. Otherwise it errors out.

[markciecior]

Hi @emichaud - Thanks for the PR. While I agree your approach would work, what do you think about my alternative? Every API module would still need to be touched in both cases, but maybe a new method isn't needed in each class?

[emichaud]

Hi Mark - took a look your approach. I tried to get it working but I kept getting errors for missing the value parameter when I used a signature like api.update_ticket(12345, change_dict). I'm sure that would be fixed.

My first thought was to modify the existing pattern (like you did) but without making a breaking change. I wasn't sure how to do that cleanly, so I figured the next best thing was to just start with a new pattern to support one or more updates via a change dict - which is probably how it should have been done initially. So I figured add an approach that works for one or many changes and then maybe deprecate the original update method.

But, I didn't feel good about suggesting deprecating the original method because it's actually a really simple yet expressive way to update a ticket. so I'd like it keep it actually.

So that comes back to your approach, if you get it working so it supports both signatures: update_ticket(1234, "summary", "this is a new summary") or update_ticket(1234, changes_dict)

then that would accomplish both our goals and avoid adding a new method to every endpoint which is smart I think. We just would have to be clear in the documentation so it's easy to discover the new approach.

Hope that feedback helps and I'm happy either way if we get the new feature in soon :). If you go with updating the update_ticket method, I'd be happy to update the endpoints with the new syntax and push them up. I appreciate the discussion.

[emichaud]

Hi @markciecior, just checking in. What are your thoughts on next step? What can I do to help?

[emichaud]

Hey @markciecior,

I worked on this a bit more to try and figure out the method signatures. Here is a quick test.

-- add a 3rd parameter to the signature, make all optional

def update_ticket(ticketId:int, key:str=None, value=None, value_dict:list=None):
    if value_dict:
        print(value_dict)
    else:
        print(f'Ticket: {ticketId}, Key: {key}, Value: {value}')

-- simulate a list of changes changes = [{'op':'replace','summary':'New Summary'},{'op':'replace','status/id':24}]

--try different signatures

--original signature update_ticket(12345, "summary", "server is down!") Ticket: 12345, Key: summary, Value: server is down! ** appears to be backwards compatible

-- try with all fields populated update_ticket(12345, "summary", "server is down!", changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** printed list due to logic, that would also work

--try with only a list of changes update_ticket(12345, changes) Ticket: 12345, Key: [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}], Value: None ** this does not work as intended as the "key" grabs the list for the first parameter

--try with a named parameter update_ticket(12345, value_dict=changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** this works using the named parameter

Now, I'm kinda new to this so I might be missing something obvious. but if my approach is correct and the named parameter is the only way to get this working, then I would have to change my recommendation back to my initial idea of just adding a new method for the multi updates.

rationale: we could get this to work and be backwards compatible, but we would also be forcing the method to do two things and create a dependency on the named parameter. If that is the case, it might be the only place where that type of syntax is required.

I guess I'd prefer to: a) add the new method (maintain single concern for each method) b) point the original update by key and value to just wrap the new method by calling it with a prepared list[dict] c) consider deprecating the original method and point to the new method for future use

I'm struggling a bit with how to infer a list of dictionaries as a parameter, maybe type hinting? In either case, it's mostly semantics.

Your call, just let me know what i can do to help.

[emichaud]

Hey @mendykrauss,

I'm working with Mark on a pull request to add that type of functionality to the project. Solving the problem is not hard, just deciding on the best way to solve it takes some thought because once it becomes part of the code base, it has to be maintained.

I still need the update functionality for a project I'm working right now, so as a work around I came up with a subclass approach that you can use with the current shipping version of ConnectPyse.

Its a single file custom controller that only supports purchase orders and service ticket updates - purchase orders for you and service tickets for me, but nothing else. Usage will feel very similar to existing methods.

You can download the code here: https://gist.github.com/emichaud/83b9cd6b186c19c1087d4c5e90383e48

Here is an example of how to send an update with multiple changes in one pass. `` from custom_update_controller import CustomPurchaseOrderAPI

connect to api

api = CustomPurchaseOrderAPI(url=BASE_URL, auth=AUTH)

use pattern of dict with many key/value pairs

mychanges = {"shippingInstructions":"place on front porch" , "vendorOrderNumber": 8675309}

call api and update item

api.update_item_attributes(po_number, mychanges)

``

hope you find that helpful custom_update_controller.py.zip .

[mendykrauss]

I think that for now, I would keep the original update and add another update method to allow for custom dicts, in order to allow for dicts & lists...

What do you guys say?

[markciecior]

Hey @markciecior,

I worked on this a bit more to try and figure out the method signatures. Here is a quick test.

-- add a 3rd parameter to the signature, make all optional

def update_ticket(ticketId:int, key:str=None, value=None, value_dict:list=None):
  if value_dict:
      print(value_dict)
  else:
      print(f'Ticket: {ticketId}, Key: {key}, Value: {value}')

-- simulate a list of changes changes = [{'op':'replace','summary':'New Summary'},{'op':'replace','status/id':24}]

--try different signatures

--original signature update_ticket(12345, "summary", "server is down!") Ticket: 12345, Key: summary, Value: server is down! ** appears to be backwards compatible

-- try with all fields populated update_ticket(12345, "summary", "server is down!", changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** printed list due to logic, that would also work

--try with only a list of changes update_ticket(12345, changes) Ticket: 12345, Key: [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}], Value: None ** this does not work as intended as the "key" grabs the list for the first parameter

--try with a named parameter update_ticket(12345, value_dict=changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** this works using the named parameter

Now, I'm kinda new to this so I might be missing something obvious. but if my approach is correct and the named parameter is the only way to get this working, then I would have to change my recommendation back to my initial idea of just adding a new method for the multi updates.

rationale: we could get this to work and be backwards compatible, but we would also be forcing the method to do two things and create a dependency on the named parameter. If that is the case, it might be the only place where that type of syntax is required.

I guess I'd prefer to: a) add the new method (maintain single concern for each method) b) point the original update by key and value to just wrap the new method by calling it with a prepared list[dict] c) consider deprecating the original method and point to the new method for future use

I'm struggling a bit with how to infer a list of dictionaries as a parameter, maybe type hinting? In either case, it's mostly semantics.

Your call, just let me know what i can do to help.

After way too much thought, I like this idea the best - add a new method, use it to wrap the original, and someday deprecate the original.

@emichaud if you want to update your PR to include wrapping the original method, I'll merge it and bump PyPi.

[emichaud]

Hey Mark – I’ll take care of the pull request for all the endpoints. Look for that most likely later this week.

Best,

Everett

From: markciecior @.> Reply-To: markciecior/ConnectPyse @.> Date: Tuesday, January 25, 2022 at 10:52 AM To: markciecior/ConnectPyse @.> Cc: cocoaev @.>, Mention @.***> Subject: Re: [markciecior/ConnectPyse] Make 2 PATCHes at once (Issue #14)

Hey @markciecior,

I worked on this a bit more to try and figure out the method signatures. Here is a quick test.

-- add a 3rd parameter to the signature, make all optional def update_ticket(ticketId:int, key:str=None, value=None, value_dict:list=None):   if value_dict:          print(value_dict)   else:          print(f'Ticket: {ticketId}, Key: {key}, Value: {value}') -- simulate a list of changes changes = [{'op':'replace','summary':'New Summary'},{'op':'replace','status/id':24}]

--try different signatures

--original signature update_ticket(12345, "summary", "server is down!") Ticket: 12345, Key: summary, Value: server is down! ** appears to be backwards compatible

-- try with all fields populated update_ticket(12345, "summary", "server is down!", changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** printed list due to logic, that would also work

--try with only a list of changes update_ticket(12345, changes) Ticket: 12345, Key: [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}], Value: None ** this does not work as intended as the "key" grabs the list for the first parameter

--try with a named parameter update_ticket(12345, value_dict=changes) [{'op': 'replace', 'summary': 'New Summary'}, {'op': 'replace', 'status/id': 24}] ** this works using the named parameter

Now, I'm kinda new to this so I might be missing something obvious. but if my approach is correct and the named parameter is the only way to get this working, then I would have to change my recommendation back to my initial idea of just adding a new method for the multi updates.

rationale: we could get this to work and be backwards compatible, but we would also be forcing the method to do two things and create a dependency on the named parameter. If that is the case, it might be the only place where that type of syntax is required.

I guess I'd prefer to: a) add the new method (maintain single concern for each method) b) point the original update by key and value to just wrap the new method by calling it with a prepared list[dict] c) consider deprecating the original method and point to the new method for future use

I'm struggling a bit with how to infer a list of dictionaries as a parameter, maybe type hinting? In either case, it's mostly semantics.

Your call, just let me know what i can do to help.

After way too much thought, I like this idea the best - add a new method, use it to wrap the original, and someday deprecate the original.

@emichaud if you want to update your PR to include wrapping the original method, I'll merge it and bump PyPi.

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.Message ID: @.***>

[mendykrauss]

Thanks guys!

Pleasure working with you on this!