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.

Sign up for GitHub

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

Add filter_publishable_entities to publishing API [FC-0062] #257

Merged
merged 5 commits into from
Nov 12, 2024
Merged

Add filter_publishable_entities to publishing API [FC-0062] #257

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f39115f
feat: get_drafts and get_published functions on publishing api
ChrisChV Nov 8, 2024
9d2bf13
chore: Bump version
ChrisChV Nov 8, 2024
6009afe
Merge branch 'main' into chris/FAL-3921-get-drafts-and-get-published
ChrisChV Nov 8, 2024
8a1adeb
chore: Bump version to 0.18.0
ChrisChV Nov 8, 2024
19f542b
refactor: Add filter_publishable_entities and remove get_drafts and g…
ChrisChV Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion openedx_learning/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
Open edX Learning ("Learning Core").
"""

__version__ = "0.17.0"
__version__ = "0.18.0"
28 changes: 28 additions & 0 deletions openedx_learning/apps/authoring/publishing/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
"soft_delete_draft",
"reset_drafts_to_published",
"register_content_models",
"get_drafts",
"get_published",
]


Expand Down Expand Up @@ -493,3 +495,29 @@ def ready(self):
return PublishableContentModelRegistry.register(
content_model_cls, content_version_model_cls
)


def get_drafts(publishable_entities: QuerySet[PublishableEntity]) -> QuerySet[Draft]:
"""
Given a list of publishable entities returns current drafts.
Not all entities may have current drafts.
"""
entity_ids = publishable_entities.values_list("id", flat=True)

return Draft.objects.filter(
entity_id__in=entity_ids,
version__isnull=False,
).select_related("entity", "version")
Copy link
Contributor

@ormsbee ormsbee Nov 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking about this more recently. The entity_id__in query is reasonable for what this does, but it does get expensive for larger queries. But I think even more importantly, the fact that we'd be passing back a QuerySet[Draft] makes it harder for callers to chain queries.

Maybe something like this instead?

def filter_publishable_entities(entities: QuerySet[PublishableEntity], has_draft=None, has_published=None) -> QuerySet[PublishableEntity]:
    if has_draft is not None:
        entities = entities.filter(draft__version__isnull=(not has_draft))
    if has_published is not None:
        entities = entities.filter(published__version__isnull=(not has_published))

    return entities

Copy link
Contributor

@ormsbee ormsbee Nov 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

With the calling code from https://github.com/openedx/edx-platform/pull/35734/files looking something like:

draft_num_children = authoring_api.filter_publishable_entities(collection.entities, has_draft=True).count()
published_num_children = authoring_api.filter_publishable_entities(collection.entities, has_published=True).count()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. Updated here: 19f542b



def get_published(publishable_entities: QuerySet[PublishableEntity]) -> QuerySet[Published]:
"""
Given a list of publishable entities returns those who have a published version.
Not all entities may have current published version.
"""
entity_ids = publishable_entities.values_list("id", flat=True)

return Published.objects.filter(
entity_id__in=entity_ids,
version__isnull=False,
).select_related("entity", "version")
76 changes: 76 additions & 0 deletions tests/openedx_learning/apps/authoring/publishing/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,82 @@ def test_get_entities_with_unpublished_changes(self) -> None:
# should not return published soft-deleted entities.
assert len(entities) == 0

def test_get_drafts(self) -> None:
n = 13
x = 5

for i in range(n):
# Create entities with drafts
entity = publishing_api.create_publishable_entity(
self.learning_package.id,
f"entity_{i}",
created=self.now,
created_by=None,
)

publishing_api.create_publishable_entity_version(
entity.id,
version_num=1,
title=f"Entity_{i}",
created=self.now,
created_by=None,
)

for i in range(x):
# Create entities without drafts
entity = publishing_api.create_publishable_entity(
self.learning_package.id,
f"entity_no_drafts_{i}",
created=self.now,
created_by=None,
)

drafts = publishing_api.get_drafts(PublishableEntity.objects.all())
assert len(drafts) == n

def test_get_published(self) -> None:
n = 13
x = 5

for i in range(n):
# Create entities to publish
entity = publishing_api.create_publishable_entity(
self.learning_package.id,
f"entity_{i}",
created=self.now,
created_by=None,
)

publishing_api.create_publishable_entity_version(
entity.id,
version_num=1,
title=f"Entity_{i}",
created=self.now,
created_by=None,
)

publishing_api.publish_all_drafts(self.learning_package.id)

for i in range(x):
# Create entities without publish
entity = publishing_api.create_publishable_entity(
self.learning_package.id,
f"entity_no_drafts_{i}",
created=self.now,
created_by=None,
)

publishing_api.create_publishable_entity_version(
entity.id,
version_num=1,
title=f"Entity_{i}",
created=self.now,
created_by=None,
)

published = publishing_api.get_published(PublishableEntity.objects.all())
assert len(published) == n

def _get_published_version_num(self, entity: PublishableEntity) -> int | None:
published_version = publishing_api.get_published_version(entity.id)
if published_version is not None:
Expand Down