- Issue created by @pameeela
- Status changed to Needs review
about 2 months ago 2:05am 13 September 2024 - π¦πΊAustralia pameeela
No rush on this, if the demo folks would rather wait to commit this until after so it matches the recordings, that's fine!
- Status changed to Needs work
about 2 months ago 4:16am 16 September 2024 - π¦πΊAustralia pameeela
This is failing, I think because core expects there to be a blog content type, but have not had a chance to investigate.
- Status changed to Closed: duplicate
about 1 month ago 11:00pm 18 September 2024 - Status changed to Needs work
about 1 month ago 11:02pm 18 September 2024 - π¦πΊAustralia pameeela
Need coffee, not done, just rebuilt with this branch checked out.
- Assigned to L_VanDamme
- π¦πΊAustralia pameeela
@l_vandamme are able to look at this? Not sure if this is relevant also, since we will remove this field anyway? #3475460: Remove extra SEO description from blog post β
Hi @pameeela, we have a fixture that auto-populate content-type blog with data and that fixture live outside drupal_cms_blog
drupal_cms/content/node/e939b485-ca10-4061-9036-3d6d394d7214.yml ... field_description: - value: "Imagine traveling to a place no one has ever been before, far away from home. That's what space exploration is all about. We want to learn more about planets, moons, and stars, and see what secrets they hold." ...
in order to remove drupal_cms_blog, we need to move that file from drupal_cms into drupal_cms_blog
More issues that will need to be resolved before removing drupal_cms_blog from the default recipe:
- move config > actions > node.type.blog to drupal_cms_blog, currently it reside in drupal_cms
- same for config > actions > user.role.content_editor > grantPermissionsForEachTaxonomyVocabulary, because it depend on core/recipes/tags_taxonomy which is part of drupal_cms_blog (This prevent running into undefined plugin issue)
- π¦πΊAustralia pameeela
Yes, those changes are already done on the branch.
- π¦πΊAustralia pameeela
Hmm, the tests are now failing. But the recipe applies fine, so not sure what's up with that.
- πΊπΈUnited States phenaproxima Massachusetts
The problem is that drupal_cms_blog has config actions targeting the content_editor role, but that doesn't exist because the recipe doesn't require it. The solution is to add
core/recipes/content_editor_role
to therecipes
list inrecipe.yml
. - πΊπΈUnited States phenaproxima Massachusetts
A few more points of feedback.
Hi !
I applied @phenaproxima suggestions, I also added
drupal_cms_blog
to project_browser settings- πΊπΈUnited States phenaproxima Massachusetts
Tests are still failing; see #14 for why.
- First commit to issue fork.
- π§πͺBelgium L_VanDamme
@phenaproxima I removed the dependency on the content editor role. I guess there is not really a reason for a blog feature to be opinionated on user roles anyway. Also removed a leftover dependency in the drupal_cms content item.
Can you have a look again?
- πΊπΈUnited States phenaproxima Massachusetts
I see no problems with this MR as it stands. Tests pass.
@l_vandamme pointed out that the test coverage is a bit weak now because we're not actually testing that these recipes can apply on top of the base recipe, and that's a great, great point. He will open a separate issue for me to address that problem.
In the meantime, though, this is ready to go.
-
phenaproxima β
committed a765d7cb on 0.x authored by
pameeela β
Issue #3474053 by pameeela, boulaffasae, l_vandamme, phenaproxima:...
-
phenaproxima β
committed a765d7cb on 0.x authored by
pameeela β
- πΊπΈUnited States phenaproxima Massachusetts
Yay! Merged into 0.x. Great to have this done - thank you, everyone!
Automatically closed - issue fixed for 2 weeks with no activity.