Skip to content

Fix offset calculation to avoid losing offset record updates. Thanks to @HyprCoder for the fix.#542

Merged
jonathanstegall merged 3 commits intoMinnPost:masterfrom
CTMobi:fix-offset-calculation
Oct 9, 2024
Merged

Fix offset calculation to avoid losing offset record updates. Thanks to @HyprCoder for the fix.#542
jonathanstegall merged 3 commits intoMinnPost:masterfrom
CTMobi:fix-offset-calculation

Conversation

@HyprCoder
Copy link
Copy Markdown
Contributor

What does this Pull Request do?

This PR fixes the issue #541 by changing how the offset is calculated to avoid losing record updates when pulling data from SalesForce. I made the following changes:

  • Added a a third param to get_pull_query function: when true the offset for the new query is not calculated. This way when calling get_pull_query for the first time during the object_sync_for_salesforce_pull_check_records action the offset is not added two times.
  • increment_current_type_datetime is called when the does_next_offset_have_results variable is true. I made this change following this logic:
  1. after writing the items to the queue, another query is sent with an additional offset to check if there are results, using the current LastModifiedDate.
  2. If there are results, store in db the LastModifiedDate of the last item processed during the first query.
  3. the get_pull_query gets called a third time. Now if the LastModifiedDate is equal to the previous one the correct offset is used, otherwise the offset gets removed and the condition uses the last value.

How do I test this Pull Request?

  • Map some entities between WordPress and Salesforce
  • Update some data in Salesforce
  • Make some pull operations and check if data is saved.

It will be saved to db if the query has results with an additional
offset. This way we do not lose records found on check_offset_query
@jonathanstegall jonathanstegall added fix needs review A fix is in and it needs further testing bug fix Pull request that fixes a bug labels Oct 8, 2024
@jonathanstegall
Copy link
Copy Markdown
Member

@HyprCoder thanks for this, I'll review/test it in the next few days. Just at first glance, this seems like it would be just a patch release, right (version 2.2.11)? I wouldn't mind making it a 2.3 but it doesn't seem that major? I don't notice any migration issues or anything like that.

@jonathanstegall
Copy link
Copy Markdown
Member

I think it's likely that this pull request will also fix #341. It seems to me that it's likely that this was the same issue that person was experiencing.

@jonathanstegall jonathanstegall added patch pull request that requires a patch release, ex v2.1.2. This is the default for new releases. and removed fix needs review A fix is in and it needs further testing labels Oct 9, 2024
@jonathanstegall jonathanstegall added this to the v2.2.11 milestone Oct 9, 2024
@jonathanstegall jonathanstegall changed the title Fix offset calculation Fix offset calculation to avoid losing offset record updates Oct 9, 2024
@jonathanstegall jonathanstegall merged commit 5928b9f into MinnPost:master Oct 9, 2024
@jonathanstegall jonathanstegall changed the title Fix offset calculation to avoid losing offset record updates Fix offset calculation to avoid losing offset record updates. Thanks to @HyprCoder for the fix. Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix Pull request that fixes a bug patch pull request that requires a patch release, ex v2.1.2. This is the default for new releases.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants