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

Fix sync - updating source string from VCS not working #3506

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

haodave
Copy link
Contributor

@haodave haodave commented Jan 3, 2025

After source string changed in the git repo, a project sync won't update the source string in the Pontoon project. Tested with a json formatted source file.

@flodolo
Copy link
Collaborator

flodolo commented Jan 3, 2025

Can you add more details on the issue/behavior you're trying to fix, so that it can be reproduced?

Copy link
Member

@eemeli eemeli left a comment

Choose a reason for hiding this comment

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

This needs to include a test case that fails with the current code, but passes with the changes. It's not at all clear what entity change is not getting picked up by the current code.

There are also two parts to this change: How differences are identified, and whether the previous or next entity is used in Entity.objects.bulk_update(). Are both changes required?

"source",
"group_comment",
"resource_comment",
"context",
Copy link
Member

Choose a reason for hiding this comment

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

Why is context dropped?

Comment on lines +336 to +338
def entity_update(
current: Entity, update_from: Entity, fields: list[str]
) -> Optional[Entity]:
Copy link
Member

Choose a reason for hiding this comment

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

This is replacing the only use of entities_same() with a new function; we should not leave orphaned code lying around.

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.

3 participants