Skip to content

Eliminate repated sync of developer/users with no reason #1153

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

Draft
wants to merge 5 commits into
base: 4.x
Choose a base branch
from

Conversation

mxr576
Copy link
Contributor

@mxr576 mxr576 commented Jun 18, 2025

Closes #1152

TODO

  • Code clean up would be nice but the code as-is should also work

@mxr576 mxr576 changed the title Eliminate repeated update of Drupal users Eliminate repated sync of developer/users with no reason Jun 18, 2025
@mxr576
Copy link
Contributor Author

mxr576 commented Jun 18, 2025

Okay, so the same trick is not going to work inside \Drupal\apigee_edge\Job\DeveloperCreateUpdate::executeRequest() because the last modified date on the developer is a immutable and calculated on the server (as it should), so we either store the last sync date:

  • in an attribute, which could lead to sync issues when multiple Drupal instances syncs from the same Apigee instance 🛑
  • in the Drupal's database - this may work ✔️

or else...?

@mxr576 mxr576 force-pushed the issue/1152 branch 2 times, most recently from 3bf3d29 to b673014 Compare June 19, 2025 07:59
for all developer/users
@mxr576 mxr576 marked this pull request as ready for review June 19, 2025 08:12
@kedarkhaire kedarkhaire self-assigned this Jun 25, 2025
@kedarkhaire kedarkhaire self-requested a review June 25, 2025 07:03
@kedarkhaire kedarkhaire added this to the 4.0.3 milestone Jun 25, 2025
Copy link
Collaborator

@kedarkhaire kedarkhaire left a comment

Choose a reason for hiding this comment

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

Hi @mxr576
Testcases are not passing - https://github.com/kedarkhaire/apigee-edge-drupal/actions/runs/15880850942/job/44780502524#step:13:43

Please check & update the test cases also as per the changes.

Thanks!

Copy link

codecov bot commented Jul 2, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.

Project coverage is 43.58%. Comparing base (6cdd9a5) to head (a53fab0).

Files with missing lines Patch % Lines
src/Job/DeveloperCreateUpdate.php 0.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##                4.x    #1153   +/-   ##
=========================================
  Coverage     43.58%   43.58%           
  Complexity     3062     3062           
=========================================
  Files           342      342           
  Lines         11212    11212           
=========================================
  Hits           4887     4887           
  Misses         6325     6325           
Files with missing lines Coverage Δ
src/Job/DeveloperSync.php 100.00% <100.00%> (ø)
src/Job/UserCreateUpdate.php 75.67% <100.00%> (ø)
src/Job/DeveloperCreateUpdate.php 36.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mxr576
Copy link
Contributor Author

mxr576 commented Jul 4, 2025

Thanks for the notice. I need to investigate what happens and what would be a proper fix

@kedarkhaire
Copy link
Collaborator

Thanks for the notice. I need to investigate what happens and what would be a proper fix

Marking this PR to draft as testcase fix need to be implemented.

@kedarkhaire kedarkhaire marked this pull request as draft July 4, 2025 12:12
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.

drush apigee:sync always updates users
2 participants