Skip to content

Conversation

@rtibbles
Copy link
Member

@rtibbles rtibbles commented Dec 3, 2025

Summary

Add missing fields to resource details sync operation:

  • language
  • provider
  • aggregator
  • role_visibility (learner/coach)

These fields were not being synced when users selected the "Resource details" checkbox during sync operations. They are now included in the sync_resource_details field list.

Also add comprehensive unit tests to verify:

  • Each field syncs correctly when sync_resource_details=True
  • All four fields sync together
  • Fields do not sync when sync_resource_details=False

References

Fixes #4930

Reviewer guidance

🤖 This was created by Claude Code. @rtibbles then reviewed the generated output, and did iterative rounds of updates before making it ready for review 🤖

I have run the tests locally and confirmed they test what is required. Fix itself is very straightforward, simply adding the fields that had not previously been listed!

@bjester bjester self-requested a review December 16, 2025 18:44
@bjester bjester self-assigned this Dec 18, 2025
@bjester bjester changed the title Fix missing sync fields Add fields language, provider, aggregator, role_visibility to sync op Dec 18, 2025
Copy link
Member

@bjester bjester left a comment

Choose a reason for hiding this comment

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

Code quality comment

source_node_id=contentnode.node_id
)

self.assertIsNotNone(target_child)
Copy link
Member

Choose a reason for hiding this comment

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

Dead code-- Django's get() should throw ContentNode.DoesNotExist if not found. This is repeated in each test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like this was copied from some other tests in this test case, so I've cleaned it up in those too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

Add missing fields to resource details sync operation:
- language: ContentNode language field
- provider: Provider organization field
- aggregator: Aggregator organization field
- role_visibility: Visible to field (learner/coach)

These fields were not being synced when users selected the "Resource details"
checkbox during sync operations. They are now included in the sync_resource_details
field list.

Also add comprehensive unit tests to verify:
- Each field syncs correctly when sync_resource_details=True
- All four fields sync together
- Fields do not sync when sync_resource_details=False

Fixes #4930
@rtibbles rtibbles force-pushed the claude/issue-4930-plan-01HLt4SubotNV5ebAegz2jUa branch from da78d2d to f05c8e0 Compare January 6, 2026 00:10
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