reapit / foundations

Foundations platform mono repo
57 stars 21 forks source link

diff.selling variations in payloads #10079

Closed Max-Shannon closed 8 months ago

Max-Shannon commented 9 months ago

Describe the bug Some events we are receiving have a selling object in the diff object. But on a rare occurence we are seeing that in some payloads the selling property is an array.

Expected behaviour It looks like this is being malformed somehow - Note the NULL value in index 1 of the array. Is this expected behaviour? Should we be handling both structures?

Problematic Payload - Note the selling object

"diff":{
    "letting":{
      "licencing":{
        "application":[
          null,
          {
            "status":"notSet",
            "referenceNumber":null,
            "date":null,
            "granted":null,
            "expiry":null
          }
        ]
      },
      "lettingFee":[
        null,
        {
          "type":"percentage",
          "amount":0
        }
      ]
    },
    "modified":[
      "2023-10-02T15:40:59Z",
      "2023-11-02T11:37:47Z"
    ],
    "selling":[
      {
        "reservationFee":0,
        "accountPaid":null,
        "fee":null,
        "vendorId":"DLK230227",
        "publicBrochureUrl":null,
        "agencyId":"SA",
        "disposal":"privateTreaty",
        "price":18000,
        "exchangedPrice":0,
        "priceTo":null,
        "decoration":[
          "good"
        ],
        "tenure":null,
        "agency":"soleAgent",
        "recommendedPrice":0,
        "exchangedCompanyFee":null,
        "valuationPrice":0,
        "sharedOwnership":{
          "rent":null,
          "sharedPercentage":null,
          "rentFrequency":""
        },
        "completed":null,
        "agreementExpiry":null,
        "instructed":null,
        "exchangedOfficeId":"",
        "qualifier":"priceOnApplication",
        "exchanged":null,
        "status":"valuation",
        "brochureId":null
      },
      null
    ],

Other sample payload here, the selling property is not in an array. (99% of events seem to be in this format).

 "diff":{
    "modified":[
      "2023-10-26T14:14:59Z",
      "2023-10-26T14:15:11Z"
    ],
    "selling":{
      "instructed":[
        null,
        "2023-10-26"
      ],
      "status":[
        "valuation",
        "forSale"
      ]
    },
    "_eTag":[
      "\"4DDAFF510EAC414B5DEA589CBEE9E601\"",
      "\"82B95E7E811F76D236AA16FF2018C573\""
    ]
  },

I'm questioning is this a bug, or intended?

github-actions[bot] commented 9 months ago

Thank you for taking the time to report a bug. We prioritise bugs depending on the severity and implications, so please ensure that you have provided as much information as possible. If you haven’t already, it really helps us to investigate the bug you have reported if you provide ‘Steps to Replicate’ and any associated screenshots. Please ensure any personal information from the production database is obscured when submitting screenshots. This issue will be reviewed in our weekly refinement sessions and assigned to a specific project board. We may also update the ticket to request additional information, if required. For more information on our processes, please click here

plittlewood-rpt commented 9 months ago

Hi @Max-Shannon I think this is intended behaviour but I'll double check - can you let me know which customer and property id this was for so I can check the history of it? I would expect to see this happen when a property's marketing mode is switched from lettings to sales, vice versa, or when a property goes from a single marketing mode to dual marketing (ie from selling to selling AND letting). The diff object contains an array of exactly two items for each property in the payload (index 0 = new value, index 1 = old value)

In the event where a property is already a selling property, you'll see items in the diff for each property within the selling object, however in the event where a property's marketing mode changes, the ENTIRE selling object in the model is being seen for the first time, so the diff is rendered correctly (new value is the whole object, old object is the null object as before).

It looks like in your problematic example above, the property has been changed from dual marketed (selling and letting) to letting only, however I can't see the whole diff (I'd expect to see the marketingMode changing here also), but as the selling object is going from being set to null, and the letting object is only showing partial changes, that would be what I'd envisage as happened to that record. As mentioned above if you can let me know which customer and property this relates to, I can check it's history. Thanks

Max-Shannon commented 9 months ago

Hi @Max-Shannon I think this is intended behaviour but I'll double check - can you let me know which customer and property id this was for so I can check the history of it? I would expect to see this happen when a property's marketing mode is switched from lettings to sales, vice versa, or when a property goes from a single marketing mode to dual marketing (ie from selling to selling AND letting). The diff object contains an array of exactly two items for each property in the payload (index 0 = new value, index 1 = old value)

In the event where a property is already a selling property, you'll see items in the diff for each property within the selling object, however in the event where a property's marketing mode changes, the ENTIRE selling object in the model is being seen for the first time, so the diff is rendered correctly (new value is the whole object, old object is the null object as before).

It looks like in your problematic example above, the property has been changed from dual marketed (selling and letting) to letting only, however I can't see the whole diff (I'd expect to see the marketingMode changing here also), but as the selling object is going from being set to null, and the letting object is only showing partial changes, that would be what I'd envisage as happened to that record. As mentioned above if you can let me know which customer and property this relates to, I can check it's history. Thanks

Hi, @plittlewood-rpt Thank you for the response.

Yes that would be correct, this does look like it started out life originally as a selling Ad, then moved to saleAndLetting and then just to letting.

Here is the eventId etc:

  "eventId":"709fedea-24ad-4b96-9649-7145d1ce5de1",
  "entityId":"DLK230227",
  "customerId":"SFG",
plittlewood-rpt commented 9 months ago

Hi @Max-Shannon sorry for the delay in getting back to you on this, it slipped through the net. i've just checked the property above and you are correct, this is exactly what happened. I think therefore that the behaviour you've reported in the webhook diff object is as I'd expected it to be. On that basis are you happy for me to close this down? Thanks

github-actions[bot] commented 9 months ago

We have recently requested additional information relating to the issue you have raised. Please can you take the time to review this ticket and where applicable, provide the information requested. For more information on our processes, please click here