No dependency on the Node type class module

Created on 20 May 2019, about 5 years ago
Updated 3 May 2024, about 2 months ago

Hello,
This module is using node_type_class module's library and data but it does not provide dependency on this module. It generates errors like:

User warning: The following theme is missing from the file system: node_type_class in drupal_get_filename() (line 276 of core/includes/bootstrap.inc).

Could you please add a dependency or remove this module usage from the code (or add a few checking before the module's logic usage?)
Thank you.

🐛 Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

🇷🇺Russia imyaro

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • 🇺🇸United States alison

    We've got both custom_body_class and node_type_class modules on a site we're onboarding, and things get a little hinky when you try to fill in one or the other on the content type settings form.

    **Total speculation** BUT I wonder if this module was created from a copy of the node_type_class module, and some stuff got accidentally left behind, and/or some stuff didn't get "found-and-replaced" during initial module development?

    node_type_class is the name of the field on the content type settings form in the node_type_class module:
    https://git.drupalcode.org/project/node_type_class/-/blob/e880d11d2f42bc...

    So, that needs to be changed (and an update hook added).

    OR... hear me out... the per-content-type class setting could be removed from custom_body_class, and users could be advised to add node_type_class to get that functionality -- and custom_body_class could be used exclusively for per-node body classes. Ehh??

  • 🇫🇮Finland HeikkiY Oulu

    There is an open merge request to fix the issue in issue https://www.drupal.org/project/custom_body_class/issues/3026806 . It seems like that issue has been closed accidentally and cannot be reopened without the maintainer.

    We have been using the patch for a long time without problems. It seems like it still is mergeable to the latest version: https://git.drupalcode.org/project/custom_body_class/-/merge_requests/2.

  • First commit to issue fork.
  • Assigned to AstonVictor
  • Merge request !53055694 - Fix dependencies → (Open) created by AstonVictor
  • Issue was unassigned.
  • Status changed to Needs review 7 months ago
  • 🇺🇦Ukraine AstonVictor

    Created a new MR.

    Replaced 'node_type_class' settings with 'custom_body_class' in order not to have a conflict with Node type class module.

    We should also do it because getThirdPartySetting() method uses the module name as a first parameter.
    I also fixed validation functions for node/node_type forms.

  • Status changed to Needs work 7 months ago
  • 🇫🇮Finland HeikkiY Oulu

    I checked the merge request and to me it seems like it has really many unrelated and outside of scope changes.

    The original patch I mentioned in #8 only deals with the wrong dependency: https://www.drupal.org/files/issues/2019-01-18/remove-node-type-class-fe...

    The merge request here https://git.drupalcode.org/project/custom_body_class/-/merge_requests/5/... seems to do a lot more like move help texts around etc. Should these be handled in this issue because the issue title does not really contain it or would it make sense to only fix the original issue and create new issue(s) for the other fixes?

    It would make it easier to review this PR.

  • Status changed to Needs review 7 months ago
  • 🇺🇦Ukraine AstonVictor

    Created a new MR.

    Replaced 'node_type_class' with 'custom_body_class' in order to avoid conflicts with Node type class module.
    We should also use the 'custom_body_class' in getThirdPartySetting() method because it uses the module name as the first parameter.

    I also fixed validation callbacks for node/node_type forms and fixed some cs issues.
    Since we updated the storage for classes, changes should be added to a new version of the module or we should create an update function to move settings from node_type_class to custom_body_class.

    Let me know what you think about it. thanks.

  • 🇫🇮Finland HeikkiY Oulu

    I think the validation and code quality issues should have a new ticket and create the fixes there. It will be easier to track where and why the changes happened because now the issue title No dependency on the Node type class module does not cover those changes.

  • 🇺🇦Ukraine AstonVictor

    Hi @HeikkiY,

    the module should not have any dependencies. that's why I removed all code related to node_type_class.

  • 🇫🇮Finland HeikkiY Oulu

    @astonvictor I don't feel comfortable marking this as reviewed, sorry. You are for example changing the whole module description in this MR which is definitely outside of this issues scope.

    Maybe we need to wait for the maintainer to comment on this.

  • Merge request !73055694 - Fix dependecies → (Open) created by AstonVictor
  • 🇺🇦Ukraine AstonVictor

    FYI made another MR as well that only contains changes to remove all code related to the 'node_type_class'.

  • Status changed to RTBC 7 months ago
  • 🇫🇮Finland HeikkiY Oulu

    The second smaller MR looks good to me.

    +1 for RTBC for those changes.

  • 🇺🇦Ukraine AstonVictor

    hi @Jitendra verma,

    any updates? could you review and merge it?

Production build 0.69.0 2024