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!

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
@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.

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