Don't filter out expired allowances, but add metadata to response

CosmWasm · Oct 9, 2020 · 09b7b31 · 09b7b31
1 parent dd79520
commit 09b7b31
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 18 deletions.
diff --git a/contracts/cw721-base/schema/all_nft_info_response.json b/contracts/cw721-base/schema/all_nft_info_response.json
@@ -29,6 +29,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -40,6 +41,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/contracts/cw721-base/schema/approved_for_all_response.json b/contracts/cw721-base/schema/approved_for_all_response.json
@@ -18,6 +18,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -29,6 +30,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/contracts/cw721-base/schema/owner_of_response.json b/contracts/cw721-base/schema/owner_of_response.json
@@ -28,6 +28,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -39,6 +40,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/contracts/cw721-base/src/contract.rs b/contracts/cw721-base/src/contract.rs
@@ -458,19 +458,15 @@ fn query_all_approvals<S: Storage, A: Api, Q: Querier>(
 
     let res: StdResult<Vec<_>> = operators_read(&deps.storage, &owner_raw)
         .range(start.as_deref(), None, Order::Ascending)
-        .filter_map(|item| match item {
-            Ok((k, expires)) => {
-                // we remove all expired operators from the result
-                if expires.is_expired(&env.block) {
-                    None
-                } else {
-                    match deps.api.human_address(&k.into()) {
-                        Ok(spender) => Some(Ok(cw721::Approval { spender, expires })),
-                        Err(e) => Some(Err(e)),
-                    }
-                }
-            }
-            Err(e) => Some(Err(e)),
+        .map(|item| {
+            item.and_then(|(k, expires)| {
+                let spender = deps.api.human_address(&k.into())?;
+                Ok(cw721::Approval {
+                    spender,
+                    expires,
+                    is_expired: expires.is_expired(&env.block),
+                })
+            })
         })
         // FIXME: decide: take before (fix gas costs) or after (fix return value size)?
         .take(limit)
@@ -520,15 +516,19 @@ fn humanize_approvals<A: Api>(
 ) -> StdResult<Vec<cw721::Approval>> {
     info.approvals
         .iter()
-        .filter(|apr| !apr.expires.is_expired(block))
-        .map(|apr| humanize_approval(api, apr))
+        .map(|apr| humanize_approval(api, block, apr))
         .collect()
 }
 
-fn humanize_approval<A: Api>(api: A, approval: &Approval) -> StdResult<cw721::Approval> {
+fn humanize_approval<A: Api>(
+    api: A,
+    block: &BlockInfo,
+    approval: &Approval,
+) -> StdResult<cw721::Approval> {
     Ok(cw721::Approval {
         spender: api.human_address(&approval.spender)?,
         expires: approval.expires,
+        is_expired: approval.expires.is_expired(&block),
     })
 }
 
@@ -980,7 +980,8 @@ mod tests {
             ApprovedForAllResponse {
                 operators: vec![cw721::Approval {
                     spender: "operator".into(),
-                    expires: Expiration::Never {}
+                    expires: Expiration::Never {},
+                    is_expired: false,
                 }]
             }
         );
@@ -1002,6 +1003,7 @@ mod tests {
                 operators: vec![cw721::Approval {
                     spender: "buddy".into(),
                     expires: buddy_expires,
+                    is_expired: false,
                 }]
             }
         );
@@ -1018,7 +1020,8 @@ mod tests {
             ApprovedForAllResponse {
                 operators: vec![cw721::Approval {
                     spender: "operator".into(),
-                    expires: Expiration::Never {}
+                    expires: Expiration::Never {},
+                    is_expired: false,
                 }]
             }
         );
@@ -1036,6 +1039,22 @@ mod tests {
                 operators: vec![cw721::Approval {
                     spender: "buddy".into(),
                     expires: buddy_expires,
+                    is_expired: false,
+                }]
+            }
+        );
+
+        // show the is_expired flag is set at the proper time
+        let mut late_env = mock_env();
+        late_env.block.height = 1234569; // expired
+        let res = query_all_approvals(&deps, late_env, "person".into(), None, None).unwrap();
+        assert_eq!(
+            res,
+            ApprovedForAllResponse {
+                operators: vec![cw721::Approval {
+                    spender: "buddy".into(),
+                    expires: buddy_expires,
+                    is_expired: true,
                 }]
             }
         );

diff --git a/packages/cw721/schema/all_nft_info_response.json b/packages/cw721/schema/all_nft_info_response.json
@@ -29,6 +29,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -40,6 +41,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/packages/cw721/schema/approved_for_all_response.json b/packages/cw721/schema/approved_for_all_response.json
@@ -18,6 +18,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -29,6 +30,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/packages/cw721/schema/owner_of_response.json b/packages/cw721/schema/owner_of_response.json
@@ -28,6 +28,7 @@
       "type": "object",
       "required": [
         "expires",
+        "is_expired",
         "spender"
       ],
       "properties": {
@@ -39,6 +40,10 @@
             }
           ]
         },
+        "is_expired": {
+          "description": "true if already expired (for easy filtering in a client)",
+          "type": "boolean"
+        },
         "spender": {
           "description": "Account that can transfer/send the token",
           "allOf": [

diff --git a/packages/cw721/src/query.rs b/packages/cw721/src/query.rs
@@ -63,6 +63,8 @@ pub struct Approval {
     pub spender: HumanAddr,
     /// When the Approval expires (maybe Expiration::never)
     pub expires: Expiration,
+    /// true if already expired (for easy filtering in a client)
+    pub is_expired: bool,
 }
 
 #[derive(Serialize, Deserialize, Clone, PartialEq, JsonSchema, Debug)]