diff --git a/localdev/configs/ocw-course-site-config.yml b/localdev/configs/ocw-course-site-config.yml index 258c73a44..15530f830 100644 --- a/localdev/configs/ocw-course-site-config.yml +++ b/localdev/configs/ocw-course-site-config.yml @@ -199,6 +199,11 @@ collections: - label: "Extra Course Numbers (comma separated list)" name: "extra_course_numbers" widget: "string" + - label: "Hide Course Download" + name: "hide_download" + required: true + widget: "boolean" + help: "Enable this setting to hide the course download button" - label: Course Image name: course_image widget: relation diff --git a/static/js/components/PublishDrawer.test.tsx b/static/js/components/PublishDrawer.test.tsx index 0bd6df19c..324ca04bd 100644 --- a/static/js/components/PublishDrawer.test.tsx +++ b/static/js/components/PublishDrawer.test.tsx @@ -95,6 +95,7 @@ describe("PublishDrawer", () => { urlField: "draft_url", publishDateField: "draft_publish_date", publishStatusField: "draft_publish_status", + hasSiteMetaData: "has_site_metadata", idx: 0, }, { @@ -105,6 +106,7 @@ describe("PublishDrawer", () => { urlField: "live_url", publishDateField: "publish_date", publishStatusField: "live_publish_status", + hasSiteMetaData: "has_site_metadata", idx: 1, }, ])( @@ -117,6 +119,7 @@ describe("PublishDrawer", () => { urlField, publishDateField, publishStatusField, + hasSiteMetaData, idx, }) => { ;[true, false].forEach((visible) => { @@ -194,6 +197,16 @@ describe("PublishDrawer", () => { expect(wrapper.find(".btn-publish").prop("disabled")).toBe(true) }) + it("disables publish button in production if no metadata is set", async () => { + website[hasSiteMetaData] = false + const { wrapper } = await render() + await simulateClickPublish(wrapper, action) + wrapper.update() + expect(wrapper.find(".btn-publish").prop("disabled")).toBe( + action === "production", + ) + }) + it("render only the preview button if user is not an admin", async () => { website["is_admin"] = false const { wrapper } = await render() @@ -240,9 +253,10 @@ describe("PublishDrawer", () => { const { wrapper } = await render() await simulateClickPublish(wrapper, action) wrapper.update() - expect( - wrapper.find("PublishForm").find(".btn-publish").prop("disabled"), - ).toBeFalsy() + website.has_site_metadata && + expect( + wrapper.find("PublishForm").find(".btn-publish").prop("disabled"), + ).toBeFalsy() await act(async () => { wrapper.find("PublishForm").find(".btn-publish").simulate("submit") }) diff --git a/static/js/components/PublishDrawer.tsx b/static/js/components/PublishDrawer.tsx index 04c28ddd6..313b68ee8 100644 --- a/static/js/components/PublishDrawer.tsx +++ b/static/js/components/PublishDrawer.tsx @@ -61,7 +61,9 @@ const getPublishingInfo = ( const PublishingOption: React.FC = (props) => { const { publishingEnv, selected, onSelect, website, onPublishSuccess } = props const publishingInfo = getPublishingInfo(website, publishingEnv) - + const isPublishDisabled = + !publishingInfo.hasUnpublishedChanges || + (!website.has_site_metadata && publishingEnv === PublishingEnv.Production) const [{ isPending }, publish] = useMutation( (payload: WebsitePublishPayload) => websitePublishAction(website.name, publishingEnv, payload), @@ -123,7 +125,7 @@ const PublishingOption: React.FC = (props) => { )} diff --git a/static/js/resources/ocw-course-site-config.json b/static/js/resources/ocw-course-site-config.json index ae6740099..ec8844c3a 100644 --- a/static/js/resources/ocw-course-site-config.json +++ b/static/js/resources/ocw-course-site-config.json @@ -266,6 +266,13 @@ "name": "extra_course_numbers", "widget": "string" }, + { + "help": "Enable this setting to hide the course download button", + "label": "Hide Course Download", + "name": "hide_download", + "required": true, + "widget": "boolean" + }, { "collection": "resource", "display_field": "title", diff --git a/static/js/types/websites.ts b/static/js/types/websites.ts index bfb3fa11b..6b93ec394 100644 --- a/static/js/types/websites.ts +++ b/static/js/types/websites.ts @@ -236,6 +236,7 @@ export interface WebsiteStatus { synced_on: string | null sync_errors: Array | null unpublished: boolean + has_site_metadata: boolean } export type Website = WebsiteStatus & { diff --git a/static/js/util/factories/websites.ts b/static/js/util/factories/websites.ts index 9a01cd636..db15c3840 100644 --- a/static/js/util/factories/websites.ts +++ b/static/js/util/factories/websites.ts @@ -227,6 +227,7 @@ export const makeWebsiteDetail = ( url_suggestion: "[sitemetadata:primary_course_number]-[sitemetdata:title]", s3_path: `courses/${casual.word}`, unpublished: false, + has_site_metadata: false, ...overrides, }) @@ -249,6 +250,7 @@ export const makeWebsiteStatus = ( synced_on: null, sync_errors: null, unpublished: false, + has_site_metadata: website.has_site_metadata, } } diff --git a/websites/api.py b/websites/api.py index 3f5661988..381c0f592 100644 --- a/websites/api.py +++ b/websites/api.py @@ -398,6 +398,9 @@ def get_content_warnings(website): messages = [] + if not website.has_site_metadata: + messages.append("The course is missing metadata.") + if len(missing_youtube_ids_titles) > 0: messages.append( f"The following video resources require YouTube IDs: {', '.join(missing_youtube_ids_titles)}" # noqa: E501 diff --git a/websites/api_test.py b/websites/api_test.py index cf7105848..5f7a19c75 100644 --- a/websites/api_test.py +++ b/websites/api_test.py @@ -1,9 +1,12 @@ """Tests for websites API functionality""" +from pathlib import Path from uuid import UUID import factory import pytest +import yaml +from django.conf import settings from mitol.common.utils import now_in_utc from content_sync.constants import VERSION_DRAFT, VERSION_LIVE @@ -41,6 +44,7 @@ PreviewOrPublishSuccessMessage, ) from websites.models import Website +from websites.serializers_test import VALID_METADATA pytestmark = pytest.mark.django_db @@ -487,11 +491,31 @@ def test_update_unpublished_website_status(status, version): @pytest.mark.parametrize("has_missing_captions", [True, False]) @pytest.mark.parametrize("has_truncatable_text", [True, False]) @pytest.mark.parametrize("is_draft", [True, False]) -def test_get_content_warnings( - mocker, has_missing_ids, has_missing_captions, has_truncatable_text, is_draft +@pytest.mark.parametrize("valid_metadata", [True, False]) +def test_get_content_warnings( # pylint: disable=too-many-arguments # noqa: PLR0913 + mocker, + has_missing_ids, + has_missing_captions, + has_truncatable_text, + is_draft, + valid_metadata, ): """get_content_warnings should return expected warning messages""" - website = WebsiteFactory.create() + site_config_path = "localdev/configs/ocw-course-site-config.yml" + starter = WebsiteStarterFactory( + config=yaml.load( + (Path(settings.BASE_DIR) / site_config_path).read_text(), + Loader=yaml.SafeLoader, + ) + ) + website = WebsiteFactory.create(starter=starter) + metadata = {**VALID_METADATA} + if not valid_metadata: + del metadata["course_title"] + + WebsiteContentFactory.create( + type="sitemetadata", website=website, metadata=metadata + ) video_content = WebsiteContentFactory.create_batch(3, website=website) no_yt_ids = video_content[0:2] if has_missing_ids else [] no_caps = video_content[1:3] if has_missing_captions else [] @@ -515,26 +539,36 @@ def test_get_content_warnings( ) warnings = get_content_warnings(website) warnings_len = 0 + if not valid_metadata: + warnings_len += 1 if has_missing_ids: warnings_len += 1 for content in no_yt_ids: - assert content.title in warnings[0] + assert content.title in warnings[1 if not valid_metadata else 0] if has_missing_captions: warnings_len += 1 for content in no_caps: - assert content.title in warnings[1 if has_missing_ids else 0] + assert ( + content.title + in warnings[int(not valid_metadata) + int(has_missing_ids)] + ) if has_truncatable_text: warnings_len += 1 assert ( video_content[2].title - in warnings[int(has_missing_ids) + int(has_missing_captions)] + in warnings[ + int(not valid_metadata) + + int(has_missing_ids) + + int(has_missing_captions) + ] ) if is_draft: warnings_len += 1 assert len(warnings) == warnings_len assert video_content[1].title in warnings[warnings_len - 1] if ( - not has_missing_ids + valid_metadata + and not has_missing_ids and not has_missing_captions and not has_truncatable_text and not is_draft diff --git a/websites/models.py b/websites/models.py index f67b5fda8..3c0c447bd 100644 --- a/websites/models.py +++ b/websites/models.py @@ -41,6 +41,7 @@ from websites.site_config_api import ConfigItem, SiteConfig from websites.utils import ( get_dict_field, + is_metadata_has_required_fields, permissions_group_name_for_role, set_dict_field, ) @@ -282,6 +283,21 @@ def s3_path(self): ] return "/".join([part.strip("/") for part in url_parts if part]) + @property + def has_site_metadata(self): + """ + Get True when required fields are set in WebsiteContent metadata + otherwise False + """ + site_metadata = self.websitecontent_set.filter(type="sitemetadata") + return bool( + site_metadata + and site_metadata[0].metadata + and is_metadata_has_required_fields( + SiteConfig(self.starter.config), site_metadata[0].metadata + ) + ) + class Meta: permissions = ( ("publish_website", "Publish or unpublish a website"), diff --git a/websites/serializers.py b/websites/serializers.py index aa6a5fc0a..ddf58ea7d 100644 --- a/websites/serializers.py +++ b/websites/serializers.py @@ -253,6 +253,7 @@ class WebsiteDetailSerializer( live_url = serializers.SerializerMethodField(read_only=True) draft_url = serializers.SerializerMethodField(read_only=True) unpublished = serializers.ReadOnlyField() + has_site_metadata = serializers.ReadOnlyField() def get_is_admin(self, obj): """Determine if the request user is an admin""" @@ -297,6 +298,7 @@ class Meta: "sync_errors", "synced_on", "content_warnings", + "has_site_metadata", ] read_only_fields = [ "uuid", @@ -329,6 +331,8 @@ class WebsiteStatusSerializer( ): """Serializer for website status fields""" + has_site_metadata = serializers.ReadOnlyField() + class Meta: model = Website fields = [ @@ -349,6 +353,7 @@ class Meta: "synced_on", "content_warnings", "url_suggestion", + "has_site_metadata", ] read_only_fields = fields diff --git a/websites/serializers_test.py b/websites/serializers_test.py index 44fa240c6..0ea400751 100644 --- a/websites/serializers_test.py +++ b/websites/serializers_test.py @@ -4,7 +4,9 @@ from types import SimpleNamespace import pytest +import yaml from dateutil.parser import parse as parse_date +from django.conf import settings from django.core.files.uploadedfile import SimpleUploadedFile from django.db.models import CharField, Value from mitol.common.utils import now_in_utc @@ -175,6 +177,48 @@ def test_website_status_serializer(mocker, settings, drive_folder, warnings): assert serialized_data.get(key) == value +VALID_METADATA = { + "status": True, + "course_title": "example", + "primary_course_number": "1", + "department_numbers": ["3"], + "hide_download": True, +} + + +@pytest.mark.parametrize( + "missing_field", + [ + None, + "course_title", + "primary_course_number", + "department_numbers", + "hide_download", + ], +) +@pytest.mark.parametrize( + "serializer_class", [WebsiteDetailSerializer, WebsiteStatusSerializer] +) +def test_website_content_has_metadata(mocker, missing_field, serializer_class): + """has_site_metadata should be true if we have metadata in WebsiteContent model""" + site_config_path = "localdev/configs/ocw-course-site-config.yml" + starter = WebsiteStarterFactory( + config=yaml.load( + (Path(settings.BASE_DIR) / site_config_path).read_text(), + Loader=yaml.SafeLoader, + ) + ) + website = WebsiteFactory.create(starter=starter) + metadata = {**VALID_METADATA} + if missing_field: + del metadata[missing_field] + WebsiteContentFactory.create( + type="sitemetadata", website=website, metadata=metadata + ) + serialized_data = serializer_class(instance=website).data + assert serialized_data["has_site_metadata"] == bool(not missing_field) + + @pytest.mark.parametrize("has_starter", [True, False]) @pytest.mark.parametrize("drive_folder", [None, "abc123"]) @pytest.mark.parametrize("drive_credentials", [None, {"creds: True"}]) diff --git a/websites/utils.py b/websites/utils.py index 9acc7e472..b63952edf 100644 --- a/websites/utils.py +++ b/websites/utils.py @@ -6,6 +6,7 @@ from django.db.models import Q from websites import constants +from websites.site_config_api import SiteConfig def permissions_group_name_for_role(role, website): @@ -96,3 +97,30 @@ def resource_reference_field_filter( def is_test_site(site_name: str) -> bool: return site_name in settings.OCW_TEST_SITE_SLUGS + + +def is_required_or_min(field): + """ + Get True if field have either 'required' property set to True or 'min' + property set to > 0 + """ + return ("required" in field or "min" in field) and ( + field.get("required") or (field.get("min") and int(field.get("min")) > 0) + ) + + +def get_config_required_fields(config: SiteConfig): + """Retrieve the 'required' fields in the WebsiteStarter.""" + config_item = config.find_item_by_name("metadata") + metadata_fields = config_item.item.get("files")[0].get("fields") + return [field.get("name") for field in metadata_fields if is_required_or_min(field)] + + +def is_metadata_has_required_fields(config, metadata): + """Validate the presence of all required fields in the WebsiteContent metadata.""" + required_fields = get_config_required_fields(config) + return all( + field in metadata + and ((isinstance(metadata[field], bool) and True) or (bool(metadata[field]))) + for field in required_fields + )