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

SDSS-1300: Added Featured Events field and Feeds #477

Merged

Conversation

buttonwillowsix
Copy link
Contributor

@buttonwillowsix buttonwillowsix commented Aug 14, 2024

READY FOR REVIEW

Summary

  • Allow SDSS to have two event feeds for Mailchimp, one for featured events and one without featured events

Review By (Date)

  • August 31

Criticality

HIGH

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Create some events in the future
  3. Mark a couple as featured using the checkbox at the top of the form
  4. Check that featured events show up here: rss/featured-events-this-week
  5. Check that no featured events show up: /rss/events-this-week

Front End Validation

NO FRONT END

Backend / Functional Validation

Code

  • [ YES] Are the naming conventions following our standards?
  • [ YES] Does the code have sufficient inline comments?
  • [ NOPE] Is there anything in this code that would be hidden or hard to discover through the UI?

Code security

General

  • [Sort of ] Is there anything included in this PR that is not related to the problem it is trying to solve?
    -- This includes a bit of clean up on the editor form
  • [YES ] Is the approach to the problem appropriate?

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

  • SDSS-1300

Resources

@github-actions github-actions bot added size/s and removed size/xs labels Aug 14, 2024
@buttonwillowsix buttonwillowsix self-assigned this Aug 15, 2024
Copy link
Collaborator

@joegl joegl left a comment

Choose a reason for hiding this comment

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

LGTM. Feeds work. I have to adjust a test that is failing because the related content field got nested but once that's done I will merge.

@joegl
Copy link
Collaborator

joegl commented Aug 19, 2024

@buttonwillowsix I figured out why the test is failing and am wondering how you want to address it.

  • The Related Content field has field permissions that only allow Administrators to use it.
  • The Related Content field got moved into the "Taxonomy" fieldset and was re-named to "Taxonomy and Related Content" in this PR
  • There is a test that logs in as a Contributor and fails if it finds text that says "Related Content"
  • This test is failing because even though it can't see the "Related Content" field, it sees the "Related Content" text in the fieldset label

Since only administrators can see and use the Related Content field, I'm wondering if we should remove "Related Content" from the fieldset label (because it might be confusing to non-administrators) and whether we should un-nest the field as well.

@buttonwillowsix
Copy link
Contributor Author

@joegl I will fix those permissions. We need the Related Content field to be usable. Stay tuned!

@joegl joegl merged commit d4136ee into 4.x Aug 22, 2024
5 checks passed
@joegl joegl deleted the SDSS-1300-create-a-new-featured-events-feed-for-this-week-newsletter branch August 22, 2024 21:56
@joegl joegl changed the title SDSS-1300: Featured Events This Week SDSS-1300: Added Featured Events field and Feeds Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants