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

D8CORE-5741: Opportunities Feature #801

Merged
merged 20 commits into from
Sep 9, 2024
Merged

Conversation

buttonwillowsix
Copy link
Contributor

@buttonwillowsix buttonwillowsix commented Aug 7, 2024

READY FOR REVIEW

Summary

This includes the following items for an Opportunity content type

  • Content model
  • Taxonomy
  • Role

Review By (Date)

  • August 31, 2024

Criticality

  • VPUE funded work

Urgency

  • Normal

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Make an Opportunity
  3. Ensure you can tag it
  4. Check that it matches the content model
  5. See that it is unpublished
  6. See that you can only add it and only add taxonomy stuff if you have the Opportunity Editor role

Site Configuration Sync

  • Is there a config:export in this PR that changes the config sync directory?
  • YES YES YES YES

Front End Validation

No front-end work here!

Backend / Functional Validation

Code

  • [YES ] Are the naming conventions following our standards?
  • [YES ] Does the code have sufficient inline comments?
  • [NO ] Is there anything in this code that would be hidden or hard to discover through the UI?
  • [ NO] Are there any code smells?
  • [ NO, but I promise to look to see if I can add some] Are tests provided? eg (unit, behat, or codeception)

Code security

Affected Projects or Products

  • Opportunities
  • SDSS Stack

Associated Issues and/or People

Resources

@@ -0,0 +1,28 @@
uuid: 6bd22067-d9e5-43ac-9206-6c511941d713
Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to find a better solution than 10 separate taxonomies. It makes is very difficult to do much with it and it's very difficult for content editors to know which taxonomy they need to edit.
My idea is to use a single vocabulary that is multiple levels. The top most level is the "Label" and the children will be treated like the field values. I'll have to look to see if there is any contib module for the form UI, but I could probably build one that could do that as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

@buttonwillowsix what do you think about this behavior? I've got an approach where the field form is built by the taxonomy hierarchy. The parent terms are treated like the field labels, and then a user can structure their content as they see fit. If they need 2 categories, they just create 2 parent terms. If they need 20, they create 20 parent terms. There's really no limitation to the number of vocabularies with this approach.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pookmish. I love this approach. MUCH better.

Not that this should stop us, but do you think it would be possible to migrate cardinal service vocabs (which is what is driving this insane list) into that model?

So, to accept this PR, would I just trim it down to a single "Tags" vocab? I will make that change.

Copy link
Contributor Author

@buttonwillowsix buttonwillowsix Aug 20, 2024

Choose a reason for hiding this comment

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

@pookmish I trimmed this down. Please have a look!

I also changed Course Code to allow multiple entries since there will be cross-listings for some.

@pookmish pookmish force-pushed the d8core-5741-opportunities-feature branch from 2b9c8b0 to 379d2ef Compare September 4, 2024 21:51
@github-actions github-actions bot removed the patch label Sep 4, 2024
@pookmish pookmish merged commit 63c43a0 into 11.x Sep 9, 2024
6 of 9 checks passed
@pookmish pookmish deleted the d8core-5741-opportunities-feature branch September 9, 2024 16:30
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