Draft: Functionality to add shopping items to basket from IDA.

WIP: Users can now add one or more shopping items to their basket. No validation has been implemented. this should probably be delegated to archive-specific plugins.

shopping_client/shopping_client.py

@@ -36,9 +36,13 @@ class shopping_client:
         self.connectors = connectors
 
         self.basket = None
+        self.username = None
 
     def get_basket(
-        self, convert_to_pandas: bool = False, reload: bool = False, filter_archives: bool = False
+        self,
+        convert_to_pandas: bool = False,
+        reload: bool = False,
+        filter_archives: bool = False,
     ) -> Union[list, pd.DataFrame, None]:
         """Retrieve the shopping basket for a user.
         Prompts for access token if one was not supplied to constructor.
@@ -87,6 +91,56 @@ class shopping_client:
 
         return self.basket
 
+    def add_to_basket(self, items: list):
+        """Add items to user's shopping basket.
+
+        Parameters
+        ----------
+        items : list
+            List of objects, each containing the data for a single
+            shopping item. Objects should be JSON-serializable. Examples
+            include Python `dict`s or `list`s.
+
+        Returns
+        -------
+        bool
+            `True` if items were successfully added otherwise `False`.
+        """
+
+        url = urllib.parse.urljoin(self.host, shopping_client.endpoint)
+
+        if self.username is None:
+            # Retrieve username
+            response = requests.get(url, headers=self._request_header())
+            if response.ok:
+                self.username = json.loads(response.content)["results"][0]["user_name"]
+            else:
+                warn(
+                    f"Unable to retrieve username from {self.host}; is your key valid?"
+                )
+                return False, response
+
+        # PATCH not working properly need to add existing basket items
+        payload = {
+            "shopping_cart": [
+                json.loads(item["item_data"]) for item in self.get_basket()
+            ]
+            + items
+        }
+
+        # trailing "/" required for PATCH
+        url = urllib.parse.urljoin(url, self.username) + "/"
+
+        response = requests.patch(url, json=payload, headers=self._request_header())
+        if response.ok:
+            print(
+                f"{len(items)} item{'s' if len(items) > 1 else ''} successfully added."
+            )
+            return True
+        else:
+            warn(f"Unable to add data to basket at {self.host}; is your key valid?")
+            return False, response
+
     def _request_header(self):
         while self.token is None:
             self._get_token()
@@ -102,12 +156,14 @@ class shopping_client:
                 item_data = json.loads(item["item_data"])
 
                 for connector in self.connectors:
-                    if "archive" in item_data and item_data["archive"] == connector.archive:
+                    if (
+                        "archive" in item_data
+                        and item_data["archive"] == connector.archive
+                    ):
                         filtered_items.append(item)
 
         return filtered_items
 
-
     def _basket_to_pandas(self):
         if len(self.connectors):
             converted_basket = {

[John Swinbank]

I'm wondering about the semantics of this.

As I read the code (I confess, I've not tested it directly), the user will normally get a list (or a DataFrame) by calling get_basket(). Isn't the natural thing for them to do to simply manipulate that list — adding, removing, altering items — and have those changes synced back to ESAP (either automatically, or on demand)?

Maybe easier to put that in pseudocode. The current workflow would look something like this:

basket = shopping_client.get_basket()
new_basket_entry = "A thing what I made up"
shopping_client.add_to_basket([new_basket_entry])

I guess what seems weird in the above API is that I'm manipulating the contents of the shopping basket by calling methods on some other objects.

I'm imagining an API that looks more like:

basket = shopping_client.get_basket()
basket.append("A thing what I made up")
shopping_client.sync()

[Hugh Dickinson]

This is possible but it would require some refactoring. Basically the basket would have to be implemented as its own class that provides an append method. Not sure how to expose the underlying data container DF/list either. Some of the plugins may rely on certain behaviour. Tricky.

[John Swinbank]

Is the basket not a list? (Leaving aside DataFrames for the moment for simplicity). Maybe not; I should look in my Copious Free Time.

[Hugh Dickinson]

If the user doesn't set the convert_to_pandas argument True in get_basket() then the internal representation is just a list, yes. Otherwise, it's a pandas data frame. Notwithstanding, I think what you're talking about would involve making a Basket class with a list-like interface, or which at least implements append(). Plus I'm assuming you'd either want append to immediately update the basket entry in the database or to have a save() or commit() method too (and something like a to_pandas() method for users who do want a pandas data frame to work with). It's not difficult to do that, but it is a major API change and I'm not sure if any of the plugins developed so far rely on the convert_to_pandas functionality. It might also be worth asking the various archive plugins to provide a make_basket_entry() method to avoid the hacking that Nico mentions in esap-general#68 (comment 23686)

[John Swinbank]

Hi Hugh — sorry for leaving this idle for so long.

Basically, yes, I think your summary of my requests is about right. I'm worried that the API currently in this MR is confusing to the naive user as to what's going on. The fact that I can do

>>> len(shoppingClient.get_basket())
2
>>> shoppingClient.get_basket().append("Some data")
>>> len(shoppingClient.get_basket())
3

without actually adding anything to the shopping basket seems weird; worse is that if I then try to add something using the “correct” method it explodes:

>>> shoppingClient.add_to_basket("some data")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jds/Projects/ASTRON/src/git.astron.nl/astron-sdc/esap-userprofile-python-client/shopping_client/shopping_client.py", line 125, in add_to_basket
    "shopping_cart": [
  File "/Users/jds/Projects/ASTRON/src/git.astron.nl/astron-sdc/esap-userprofile-python-client/shopping_client/shopping_client.py", line 126, in <listcomp>
    json.loads(item["item_data"]) for item in self.get_basket()
TypeError: string indices must be integers

My fairly off-the-cuff suggestion is that ShoppingClient should be a MutableSequence which stores the “raw” basket as a list of JSON strings internally. A get_as_pandas method could return a copy of the data as a dataframe. I can think more about this and write down a concrete proposal if that would be helpful (but not today...).

I think it's better to break the API now, before we have any widespread deployments; changes will only get more painful with time.

Thoughts welcome! Please push back if you think I'm crazy... :-)

[Hugh Dickinson]

Hi John,

I don't think it will be difficult to implement what you want but a concrete proposal would indeed be useful. No need to have it immediately though :-)

Cheers,

Hugh

[John Swinbank]

Last line — return False, response — doesn't match this signature.

[Hugh Dickinson]

Good catch - left over debugging code :-)

[John Swinbank]

Not sure I understand this comment.

[Hugh Dickinson]

Basically, I think it should be possible to just add to the list of shopping items using patch, but in fact one has to replace the original with a complete updated version. This is probably expected behaviour, but I wasn't expecting it. Maybe because the list of shopping items is a single field in the user profile model.

[John Swinbank]

Ah! You're thinking of an HTTP PATCH? I was reading this as something like a list.patch method in Python, and there isn't one, and I was confused. Ok, that makes sense.