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

Course publish without metadata #2015

Closed
wants to merge 15 commits into from

Conversation

Anas12091101
Copy link
Contributor

@Anas12091101 Anas12091101 commented Nov 1, 2023

What are the relevant tickets?

Fixes #1996

Description (What does it do?)

You can add content to a course and publish it in staging without ever visiting the Metadata section. Production, however will require metadata to be set.

Screenshots (if appropriate):

Screenshot 2023-10-20 at 2 34 36 PM Screenshot 2023-10-20 at 2 35 37 PM Screenshot 2023-10-20 at 2 35 51 PM

How can this be tested?

  • Clone and checkout to this branch from my forked repo
  • Restart studio containers
  • Go to the studio website at http://localhost:8043/admin and login as admin
  • Create a new site with ocw-course starter
  • (Optional) Add pages to the website
  • Open the publish drawer, you can see that the publish button is enabled in the staging and disabled in production (as we have not set metadata yet)
  • Press the publish button in staging
  • After the pipeline finishes, you get the published website on the url

Additional Context

The course-v2/layouts/partials/course_banner.html file was changed in the ocw-hugo-themes repository. You can view the changes here

Copy link
Contributor

@HussainTaj-arbisoft HussainTaj-arbisoft left a comment

Choose a reason for hiding this comment

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

Can we add or update the tests in websites/serializers_test.py to ensure that a value for has_site_metadata is serialized and present?

Similarly, should we add or update tests in static/js/components/PublishDrawer.test.tsx?

Copy link
Contributor

@HussainTaj-arbisoft HussainTaj-arbisoft left a comment

Choose a reason for hiding this comment

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

Can we also add a warning message in websites/api.py get_content_warnings?

For example

    if not website.has_site_metadata:
        messages.append(
            "The course is missing metadata."
        )

website.has_site_metadata is a property on the model class. Just for the sake of reusability.

static/js/components/PublishDrawer.test.tsx Outdated Show resolved Hide resolved
static/js/types/websites.ts Outdated Show resolved Hide resolved
websites/api.py Outdated Show resolved Hide resolved
websites/api.py Outdated Show resolved Hide resolved
websites/serializers.py Outdated Show resolved Hide resolved
websites/serializers_test.py Outdated Show resolved Hide resolved
websites/serializers_test.py Outdated Show resolved Hide resolved
websites/api.py Outdated Show resolved Hide resolved
@HussainTaj-arbisoft
Copy link
Contributor

I will reopen it to trigger CI.

@Anas12091101
Copy link
Contributor Author

@HussainTaj-arbisoft Can I close this PR?

Copy link
Contributor

@ibrahimjaved12 ibrahimjaved12 left a comment

Choose a reason for hiding this comment

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

At first I was not able to publish the course.

I was getting this error:
image

But then I noticed that this is the same file for which we have a PR here:
mitodl/ocw-hugo-themes#1274

I tried testing this PR by changing OCW_HUGO_THEMES_GIT in constants.py as you shared, to reflect the forked branch above and then upserted a new themes pipeline, and backpopulated a course.

It then worked to publish. Can you please confirm both of the PRs are supposed to be reviewed, and they're supposed to work together?

If so, can you please make sure the checks are not failing and the PR is ready to be reviewed.

This is the error:

Run shimataro/ssh-key-action@v2
  with:
    if_key_exists: replace
    name: id_rsa
✅SSH directory "/home/runner/.ssh" has been created successfully.
Error: Error: Input required and not supplied: key

@ibrahimjaved12
Copy link
Contributor

Recreated the PR here without fork, to get all checks up and running: #2183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sites without metadata fail to publish
4 participants