-
Notifications
You must be signed in to change notification settings - Fork 29
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
No apps block has no 'create' link #45
Comments
Please remove me from your mail list
Sophia Bergmann | Software Engineer
…On 18 Apr 2023 at 18.46 +0200, andrew farquharson ***@***.***>, wrote:
Description
If you navigate to /admin/structure/views/view/applications and select the application view's page display and there are no applications displayed there is a block with machine name 'noappsblock' that is displayed. This is configured in the view under 'No Results Behaviour'.
Test
To test this you can add an arbitrary filter criteria to the view that will force it to return no applications. The you will see the 'noappsblock' in the view preview.
The issue
The text states, 'Click Create to create an application.' but there is no 'create' link. This problem was reported by an admin user who could see no way to create an application. There was no link to do it the menus in the portal nor any visible links. So they would have had to guess to add '/new' to the end of the '/applications' path to reach the application configuration form.
It would seem that the intention behind the block is to make the word 'create' a link to the path '/applications/new'.
The fix
In my case the fix was to use a custom block I had already created. I tried to do that by changing the machine name in the 'No Results Behaviour' of the view to the machine name of our custom block. But for some reason that just led to a blank white screen as output for the view. But that I may refer to in a separate issue.
To fix this issue with the IBM block, i suggest instead to make the word 'create' in the text of the noappsblock block into a link to the path '/applications/new'.
The code
I was expecting to find the 'noappsblock' defined as a plugin inside the ibm_apps module at ibm_apps/src/plugin/Block/ and it took some head-scratching before i found it here: https://github.com/ibm-apiconnect/devportal/blob/master/ibm_apim/ibm_apim.emptycontent.inc
I have a feeling that this .inc file is Drupal 7 legacy code. .inc files were best practice in Drupal 7 e.g. for creating views in custom modules, for example. It reduced the memory footprint. It seems that several versions of the block are created to cater for several language translations. It makes it a bit confusing to decide which block to select in block admin as there are several defined blocks that seem to be identical.
In Drupal 7 I believe you had to do this because only the title of blocks was translatable. But now the body and the title of blocks are translatable. There appears no need to define several different versions of a block, therefore. I think it worth trying to move the code for the block out of the .inc file into a block class inside ibm_apps module and to remove the code for this block from the .inc file. I am not sure that it is easy or even possible to implement the link inside the .inc file whereas I know it will be simple to do it in the build method of a block plugin. A PR will follow soon.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
@missbergmann this is a GitHub repository not a mailing list. I think you must have clicked the option to follow this repository which would mean you get emails when things happen. You’ll need to unfollow the repository from the repository homepage https://github.com/ibm-apiconnect/devportal to stop the emails. |
@andrewfandrew I hope you know that the portal admin cannot create applications. It is true that the app block does not include a create link - it never has, I am not sure that in itself is really a defect though. |
@chrisdudley Yes I was approached by Shashank. Shashank has been using one of his accounts for testing and I share the same test account. It is a non-admin account. Both of us also have admin accounts of course. He uses the non-admin account to test out consumerorganisation and application creation. He and another user who is equally knowledgeable on permissions had discovered the noappsblock (that is its machine name) showing up when they deleted all their applications. They wanted an RMG branded block instead and they specified a block which has a link to create applications in it which i had created already. I have been testing all along with Shashank's account and deleting and creating apps to recreate the issue he wanted fixed. Interesting that you bring up the access checking which is implemented in block code as I have been studying the access checking code in particular in the consumer organisation block. I was thinking that if i do put a 'create' link in the block i will need to do the access checking to make sure the user has access to the route for creating applications. You have clarified the permissions for me and I will probably try to borrow the access checking code used in the consumer organisation block. "I am not aware of any customers actually using the blocks that come with each of our views (apps, apis and products)". I think that the blocks only appear when the user has for example not yet created an application but they can do so or when they delete all their applications. The view is configured to display the block if the view returns no results. So i think your customers are seeing the blocks but only when no results are returned by the respective views (apps, apis and products). We have one catalog/ environment just now with no customisations newly created by IBM. If i as admin got to these 3x views configurations at admin/structure/views and add an arbitrary filter that will ensure force the view to return no results, I will see each of the IBM noxxxxx blocks appear in the respective view. |
If i add an arbitrary filter criteria like this (.... equal to 444444) then you will get no results for the view: And further down in the view preview area the block appears. It appears even though it is disabled in the block administration at /admin/structure/block. btw, this is the block I am studying the access checking in: I also am looking at the code in Drupal core for the system branding block. It does access checks that seem to check access to routes. It seems quite a brief way to check access. So i may try testing if the user has access to the route for creating applications. |
Aha yes you're right - sorry i was confusing the nocontent blocks from the list blocks. The list ones were created when the view was created and i don't think are really used. You should be able to override the template, use hooks to modify what is displayed though shouldn't you? You're right about the code being of older code style - it simply hasnt been converted over to src/Plugin/Block form yet as there hadn't been a compelling need. I don't think you want to get too deep into the permissions and access controls, we already have code that decides whether to display a create app link or not - its in the existing NewApplicationBlock. Just reuse that: |
Ok, I have put together some code and raised a PR. Regarding, "You should be able to override the template, use hooks to modify what is displayed though shouldn't you?" the code that is being used in the ibm_apim/ibm_apim.emptycontent.inc file does not rely on templates. In fact it takes a bit of investigation and head-scratching. The .inc file is required in the ibm_apim.install file so the blocks get created as part of the installation of the ibm_apim module. I understand that there is a hook that allows you to overwrite the content of any block if you have the block id. But since Drupal core is moving away from hooks and towards plugins etc, not a bad thing to refactor these blocks the new bog standard way of creating and installing blocks?. The .inc is included in other places like the ibm (installation) profile and the drush command. I am leaving it in there because there is still the NoBlogs block defined in it which I have not refactored. |
If you are wondering where e.g. the applications view is created it is here The 'noappsblock' is referred to in lines 5 and 321. If testing the PR you might have to remove line 5 as now the block is done as a plugin I am not sure that the it will be deteced as a dependency in this file. Line 321 is where the view configures the block to show up on no results ('empty'). |
I understand that as a general rule, if a module uses a template then that template can be overriden in the theme. So if your client creates their custom theme e.g. using the IBM utility for that then they can create templates with the same name as the 'no content' templates I have created in the PR. The versions in their theme then take precendence over the ones in the module. |
Now i realise this is an alternative way of creating the 'no content' blocks which for some users would be useful since it hides away the content of the blocks, hard-coded inside templates. They cannot then be edited in Drupal UI. Whereas the current way of installing the blocks creates them as custom blocks which can easily be edited. You just edit the blocks in the UI. I only just realised this when I looked inside our portals' custom blocks which I have not done in months since all our blocks are implemented in templates. |
Generally speaking, unless the customer knows what they’re doing and has drupal skills in-house we advise against overriding templates. We even document as such. Customising a portal is not a one-time activity. |
The PR does not override your templates. In fact right now you are not using templates for these 'no content' blocks unless it is 'block.html.twig' which will be applied by default to any block without its own specific template. During the installation of the ibm_apim module these blocks are installed as 'custom blocks'. You will find them at: /admin/structure/block/block-content. You will see that the html content of those blocks is then editable through the Drupal UI. The method of creating the blocks in my PR creates templates and places the html inside them but the html is therefore not available to edit in the Drupal UI. I would agree that the majority of your clients would probably want to be able to edit the html through the UI. Actually I notice that Drupal 9 does use .inc files so the .inc file used in ibm_apim to install the 'no content' blocks seems regular. But the best example I can find of inserting blocks on module installation and nodes with content and blocks with content (the blocks are repeated in English and Spanish) is inside a module named 'demo_umami_content' inside the Umami installation profile. You can see in that module e.g. at config/install that there that csv files are used but also .html files. But you can see that the organisation of the languages into separate folders looks good. Makes it easy to add further languages with their own content in that language and their own blocks, nodes etc. |
Description
If you navigate to /admin/structure/views/view/applications and select the application view's page display and there are no applications displayed there is a block with machine name 'noappsblock' that is displayed. This is configured in the view under 'No Results Behaviour'.
Test
To test this you can add an arbitrary filter criteria to the view that will force it to return no applications. The you will see the 'noappsblock' in the view preview.
The issue
The text states, 'Click Create to create an application.' but there is no 'create' link. This problem was reported by an admin user who could see no way to create an application. There was no link to do it the menus in the portal nor any visible links. So they would have had to guess to add '/new' to the end of the '/applications' path to reach the application configuration form.
It would seem that the intention behind the block is to make the word 'create' a link to the path '/applications/new'.
The fix
In my case the fix was to use a custom block I had already created. I tried to do that by changing the machine name in the 'No Results Behaviour' of the view to the machine name of our custom block. But for some reason that just led to a blank white screen as output for the view. That may be a side-issue connected with the way that custom blocks are registered in Drupal 9 core.
To fix this issue with the IBM block, i suggest instead to make the word 'create' in the text of the noappsblock block into a link to the path '/applications/new'.
The code
I was expecting to find the 'noappsblock' defined as a plugin inside the ibm_apps module at ibm_apps/src/plugin/Block/ and it took some head-scratching before i found it in the root of the ibm_apim module
I have a feeling that this .inc file is Drupal 7 legacy code. .inc files were best practice in Drupal 7 e.g. for creating views in custom modules, for example. It reduced the memory footprint. It seems that several versions of the block are created to cater for several language translations. It makes it a bit confusing to decide which block to select in block admin as there are several defined blocks that seem to be identical.
In Drupal 7 I believe you had to do this because only the title of blocks was translatable. This is discussed on drupal.org
D7 and D8 block translation
From D8 onwards it seems that the body and the title of blocks are translatable. There appears no need to define several different versions of a block, therefore. I think it worth trying to move the code for the block out of the .inc file into a block class inside ibm_apps module and to remove the code for this block from the .inc file. I am not sure that it is easy or even possible to implement the link inside the .inc file whereas I know it will be simple to do it in the build method of a block plugin. A PR will follow soon.
The text was updated successfully, but these errors were encountered: