Reviewed & tested by the community
Project:
Drupal Coverage Core
Version:
8.x-1.0-alpha1
Component:
User interface
Priority:
Normal
Category:
Bug report
Assigned:
Issue tags:
Reporter:
Created:
31 Oct 2016 at 11:54 UTC
Updated:
17 Dec 2016 at 16:25 UTC
Jump to comment: Most recent, Most recent file

Comments
Comment #2
joaopauloc commentedComment #3
bramdriesenComment #4
joaopauloc commentedComment #5
akashkrishnan01 commentedI would like to work on this issue, can you give me some pointers for getting started
Comment #6
akashkrishnan01 commentedComment #7
jonathan1055 commentedHi akashkrishnan,
I don't know much about how the internals of this module works, so here is what I just did to start to discover where you would fix this. I am posting it here to show how I would go about it, when you do not know where to begin. I started by trying to find when the module title was displayed:
Plugin/Block/ModulesBlock.phpbecause this loops through all modules$analysis_manager->getTitle($analysis)line which was used as the title, not the$module_manager->getTitle($module)line.node_field_dataandnode_field_revisionhold the module name.node_field_revisionwhich is displayed. This showed that the text is not derived for display, it is the actual stored db title field which is used.The next step would be to find if/how the title is manipulated when the node is created. I guess we need to take the users input and derive the actual human-readable module name. That might sound like an obvious conclusion, but it is worth laying it out here. Hope that helps you to get started ...
Comment #8
akashkrishnan01 commentedHey jonathan1055,
Thanks for the reply. But I am a newbie here, so can you be more specific about the drupal version you cloned and what exactly drupal 8.x-1.0alpha1 is?
Comment #9
bramdriesenHi akashkrishnan,
8.x-1.0-alpha1 is the latest "stable" alpha release of the module. This is the module that a typical user would install to test out the module. As a contributor you could checkout that version of the code to test the alpha version, but development should happen on the 8.x-1.x-dev branch. Make sure to checkout this page on how to create a patch after you have worked on an issue: https://www.drupal.org/node/707484. You can install the module on a clean install of Drupal 8.
Comment #10
akashkrishnan01 commentedHey BramDriesen,
Thanks for the reply, now it is easy to figure out this issue.
Comment #11
akashkrishnan01 commentedIs this issue is to change the machine name to human readable name, i.e. from dcmodulesoverview to drupal_coverage_core_module_block?
Comment #12
bramdriesenYes that's correct. Currently core modules are displayed using the machine_name, instead of the human readable name.
Comment #13
akashkrishnan01 commented@BramDriesen, thanks
Comment #14
quocnht commentedModule node saved machine name as title and use as parameter for generating build data. As seen in ModuleManager
- Can we just use Twig filter to change machine name to full name in block template?
- For core modules, I try to get name in configuration and pass to block. Patch attached.
Comment #15
bramdriesenSounds good to me, nice work so far.
Comment #16
quocnht commented@BramDriesen
For Twig filter replace '_' with ' ' blank and make it titlecased ?
Comment #17
jonathan1055 commentedHi quocnht,
Thanks for the patch and your ideas on manipulating text using filters in twig. Good stuff.
I've been thinking about this more, though, and it is a bigger problem than just how we display the text in the block. Converting from the readable name to machine name and vice versa is not the best thing in the long term - it is not robust enough to cope with cases where the title and machine name are different, for example the D7 contrib module 'Chaos Tools' has a machine name of 'ctools'. This does not work with the current codebase. I think we need to store the readable module name in the node title and have a specific machine_name field which will permanently hold the machine name, for both core and contrib module nodes. In de-coupling the machine name from the title we make is safer for future development. This also simplifies several parts of the existing code as we no longer need to use
cleanModuleName()at all. The modules are identified in the db tables by the machine name, which is safer than using the title text.I have a working version of these changes and here is a patch. I have exported the content type config files using Configuration Update which has altered the indentation from the existing code (hence the config changes appear more complex than they are. I have also moved the fields from hidden into display, as it makes sense if an admin can edit the node then they should be able to fix the values).
Summary of the changes:
This all works with my local manual testing (including Chaos Tools/ctools) but I cannot run the automated tests, so do not know if they will pass yet.
Comment #19
jonathan1055 commentedHa ha, that's funny. That single failure is due to the fact that the test I removed was the only test in that file! I did not notice that. I guess we need to add some more tests to ModuleManagerTest.php. I have added a dummy test so that we can keep the file, until we add more tests later.
Comment #20
quocnht commentedGreat work @jonathan1055,
I apply patch, diff show also the default_config_hash. Is it needed in module config?
Comment #21
jonathan1055 commentedPleased you like the work. Always a slight worry when someone replaces the work of a previous patch.
Thanks for reminding me about config hash. I do not actually know what that relates to. Is is a hash-total of the specifc config export file, or of the whole entity? What is is used for? There were values in the files before, and when I exported via Config Update. it gave the new values, so I left them in. I did not have time to research that detail, but yes we need to understand its purpose and/or whether we can remove it.
Comment #22
bramdriesenLooks okay! Worked on my local
Comment #23
jonathan1055 commentedThanks for testing, Bram. I have just checked, and the patch in #19 still applies OK after the recent commits.