agex-inc / support.requests

Shared repository to collect bug reports and new feature requests
0 stars 0 forks source link

Offline animal membership updates are not fully synced #378

Open altemeier opened 4 years ago

altemeier commented 4 years ago

Report

Timestamp

9/21/2020 10:50:30

What environment were you working in?

TEST

Is this bug also in the DEMO and/or PRODUCTION environments?

Yes

How severe is this issue?

Moderate: should fix, but don't block next release

What device were you using?

Laptop/desktop computer

What part of the system needs work?

Herd

What version of the app were you using?

20200921

Is this a bug or a new feature request?

Bug (something unexpected is happening)

In what screen/page did you find the bug?

Groups

Were you using a "native" or "web" app?

Web (using a web browser)

What was the email (or phone number) for the account you were using?

p921@agex.io

What actually happened?

The web app shows 3 animals total (including after refreshing) - Animal 5939 seems to be duplicated. Was added to group 2, but not removed from group 1

The Animal correctly shows being in Group 2, but it is listed as part of Group 1 --- See pic 2

Logout and Login again did not fix it.

The Android client shows 2 animals, consistently with the iOS client.

Email Address

enrique@agex.io

What actions did you take to try to make that happen?

Added first 2 animals offline (and added them to Group 1). Moved the animals to Group 2.

What were you generally trying to do?

Add animals to group and move to another group

If you were using the web, what browser were you using?

Chrome

Screenshots

All Groups

Group 1

joshpavlovich commented 4 years ago

Steps to reproduce in Android

  1. Logged into the DEV environment app with 'josh+owner@agex.io'
  2. Enable 'Airplane mode' on the device to simulate offline functionality.
  3. Created a group named 'Group 1'.
  4. Added 2 animals to 'Group 1'.
  5. Created a group named 'Group 2'
  6. Moved the 2 animals from step 3 to 'Group 2' via a membership change.
  7. Disable 'Airplane mode' on the device and allowed the offline events to sync with the server.

Requests and responses

--> POST https://dev.api.agex.io/group
Content-Type: application/json; charset=UTF-8
Content-Length: 332

{
  "group_id": "a3523703-f6a8-4ae6-8293-7bbea82e8919",
  "events": [
    {
      "id": "a3523703-f6a8-4ae6-8293-7bbea82e8919",
      "event_date": 1600711377960,
      "name": "Group 1",
      "owner_id": "1b8e1530-5de0-425f-ba2d-83f5b168c8e5"
    },
    {
      "animal_ids": {
        "add": [
          "f19d3361-dd1d-4726-a2a0-b3a105f81856",
          "f9cdd8df-bff3-4fbe-8d77-0157e4d6d921"
        ]
      },
      "event_date": 1600711393721
    }
  ]
}

{
  "status": 201,
  "group": [
    {
      "id": "a3523703-f6a8-4ae6-8293-7bbea82e8919",
      "name": "Group 1",
      "owner_id": "1b8e1530-5de0-425f-ba2d-83f5b168c8e5",
      "media_ids": [],
      "type": "GROUP",
      "updated": 1600711478069,
      "created": 1600711478069,
      "deleted": false,
      "locked": false,
      "animal_ids": [
        "f19d3361-dd1d-4726-a2a0-b3a105f81856",
        "f9cdd8df-bff3-4fbe-8d77-0157e4d6d921"
      ]
    }
  ]
}

--> POST https://dev.api.agex.io/group
Content-Type: application/json; charset=UTF-8
Content-Length: 332
{
  "group_id": "6d8a11e4-7b46-487f-b009-ab30c7bd18fd",
  "events": [
    {
      "id": "6d8a11e4-7b46-487f-b009-ab30c7bd18fd",
      "event_date": 1600711409425,
      "name": "Group 2",
      "owner_id": "1b8e1530-5de0-425f-ba2d-83f5b168c8e5"
    },
    {
      "animal_ids": {
        "add": [
          "f19d3361-dd1d-4726-a2a0-b3a105f81856",
          "f9cdd8df-bff3-4fbe-8d77-0157e4d6d921"
        ]
      },
      "event_date": 1600711432233
    }
  ]
}

{
  "status": 201,
  "group": [
    {
      "id": "6d8a11e4-7b46-487f-b009-ab30c7bd18fd",
      "name": "Group 2",
      "owner_id": "1b8e1530-5de0-425f-ba2d-83f5b168c8e5",
      "media_ids": [],
      "type": "GROUP",
      "updated": 1600711478058,
      "created": 1600711478058,
      "deleted": false,
      "locked": false,
      "animal_ids": [
        "f19d3361-dd1d-4726-a2a0-b3a105f81856",
        "f9cdd8df-bff3-4fbe-8d77-0157e4d6d921"
      ]
    }
  ]
}

Results

Groups in the Android app.

Groups in the web app

altemeier commented 4 years ago

Root Cause Analysis

The "cause" of this issue is that the native offline sync process moves immediately from one successful request to the next, such that animals are added to a "new" group in the API before the "old" group has been assigned to them. Because the API does not "see" an "old" group on the animal, it does not know to remove that animal from the "old" group's animal_ids array.

Potential Solutions

  1. Native apps "pause" in between sync operations to give "cascading" requests time to take effect.
    1. CON: clients should not have to "throttle" their behavior for the API.
    2. CON: the length of the "pause" would amount to a "magic" number that may or may not be right under the conditions in the API/backend at that time.
  2. The API proactively "searches" for relationships that might not have "cascaded" yet when animal memberships are adjusted.
    1. PRO: relatively simple to implement.
    2. CON: very inefficient--the API would need to query elasticsearch every time an animal is added to a collection, just to make sure it wasn't part of a multi-transfer sync.
  3. Deprecate animal_id arrays as editable properties on collections.
    1. PRO: animal_ids would still be present and searchable on collections.
    2. CON: to update an animal's membership, a client would have to update the relevant property on the animal. Without "batch" updates in the API, this could turn one request to add 10 animals into 10 requests to update each animal.
      1. BUT: "batch" update capabilities would be useful elsewhere in the platform and have been requested/considered many times before.
  4. Deprecate animal_id arrays as editable or searchable properties on collections.
    1. PRO: by removing a "cascading" relationship, we substantially simplify backend logic.
    2. PRO: even though they are not searchable, animal_ids/animals could still be included in the response objects when collections are fetched.
    3. See Point 3.ii above, which also applies here.
    4. CON: any clients relying on searches for collections by animal_ids would have to be refactored.
      1. BUT: does any client actually do this?
    5. CON: using Kibana without animal_id arrays could be more cumbersome.
      1. BUT: we now have a SQL-compatible replica of our database for these sort of searches.
brothhammer commented 4 years ago

@altemeier option 4 sounds like the "most correct" choice. By removing the animal_ids array on a collection are there any consequences to lists? Would we need to add a lists array on the animal table?

altemeier commented 4 years ago

Good question @brothhammer. Because lists are not exclusive, they do not present the same issues.

With collections/addresses, the API needs to enforce the exclusivity requirement without relying on clients to make multiple requests "the right way". Converserly, for non-exlcusive groupings, the API cannot assume that adding an animal to one grouping requires removing it from any others, and must rely on clients to communicate that intent.

In light of this, my inclination is to leave the List models and mechanics alone for now.