Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate lastSyncedDate on object create/update when syncing object #247

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

norrisng-bc
Copy link
Contributor

@norrisng-bc norrisng-bc commented Feb 7, 2024

Description

object.lastSyncedDate is now populated on object create/update (but only when this happens during an object sync operation).

Edit, Feb 27: only populate when syncing object

https://apps.nrs.gov.bc.ca/int/jira/browse/SHOWCASE-3506

Types of changes

New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

N/A

Copy link

codeclimate bot commented Feb 7, 2024

Code Climate has analyzed commit 7702747 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 66.7% (0.0% change).

View more on Code Climate.

Copy link

github-actions bot commented Feb 7, 2024

Coverage Report

Totals Coverage
Statements: 59.95% ( 2844 / 4744 )
Methods: 50.55% ( 323 / 639 )
Lines: 66.75% ( 1700 / 2547 )
Branches: 52.7% ( 821 / 1558 )

Copy link
Contributor

@TimCsaky TimCsaky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when someone runs the sync job.. we now always need to update the object.lastSyncedDate
this change to services/sync.js should do it:
image

app/src/services/object.js Outdated Show resolved Hide resolved
Copy link
Member

@jujaga jujaga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastSyncedDate column should only be populated and modified via a /sync operation (stemming from syncBucket or syncObject for example). For standard object manipulations such as objectCreate and objectUpdate, whatever the lastSyncedDate value is must be preserved and not manipulated, as the column is meant to precisely report back when an object last had an explicit synchronization operation applied. Object create and update are not able to safely assert that a complete synchronization has taken place as that is not what those endpoints are designed to do.

app/src/services/object.js Outdated Show resolved Hide resolved
app/src/services/object.js Outdated Show resolved Hide resolved
app/src/services/object.js Outdated Show resolved Hide resolved
app/src/services/object.js Outdated Show resolved Hide resolved
@norrisng-bc norrisng-bc changed the title Populate lastSyncedDate on object create/update Populate lastSyncedDate on object create/update when syncing object Feb 28, 2024
@norrisng-bc norrisng-bc force-pushed the last-synced-date branch 2 times, most recently from 33696c9 to f123815 Compare February 28, 2024 01:50
app/src/services/object.js Outdated Show resolved Hide resolved
In an earlier commit, lastSyncedDate would be updated/populated on object update/create (even when not syncing) - this is incorrect behaviour

Also:
* Have comsObject update within the syncObject transaction (`trx`)
* Update tests: sync service, object service
@jujaga jujaga merged commit db18dae into master Feb 28, 2024
16 checks passed
@jujaga jujaga deleted the last-synced-date branch February 28, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants