Problem/Motivation

I've installed the project browser devel. When i go to admin/config/development/project_browser I get the following warning: 

Warning: Trying to access array offset on int in Drupal\project_browser\Form\SettingsForm->buildForm() (line 162 of /var/www/html/repos/project_browser/src/Form/SettingsForm.php).

Drupal\project_browser\Form\SettingsForm->buildForm()
call_user_func_array() (Line: 528)
Drupal\Core\Form\FormBuilder->retrieveForm() (Line: 279)
Drupal\Core\Form\FormBuilder->buildForm() (Line: 73)
Drupal\Core\Controller\FormController->getContentResult()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 593)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 709)
Drupal\Core\DrupalKernel->handle() (Line: 19)

in addition the source type added by the sub module has no label:

list of enabled source types for project browser plus the alllow installing via ui on top. all on the settings page for project browser

and in addition to that admin/modules/browse only shows the three source types drupal.org, recipes and drupal core, the source type added by the project browser devel module has no tab.

Steps to reproduce

Proposed resolution

CommentFileSizeAuthor
#9 Screenshot from 2024-11-19 12-02-46.png42.78 KBshalini_jha
sourcetypes.jpg62.1 KBrkoller

Issue fork project_browser-3479924

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

rkoller created an issue. See original summary.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Issue tags: +core-mvp, +beta blocker
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

confirming this is still reproducible

shalini_jha made their first commit to this issue’s fork.

shalini_jha changed the visibility of the branch 3479924-project-browser-devel to hidden.

shalini_jha changed the visibility of the branch 3479924-project-browser-devel to active.

shalini_jha’s picture

shalini_jha commented
Assigned: Unassigned » shalini_jha

shalini_jha opened merge request !621

shalini_jha’s picture

shalini_jha commented
Assigned: shalini_jha » Unassigned
Status: Active » Needs review
StatusFileSize
new Screenshot from 2024-11-19 12-02-46.png42.78 KB

I was able to replicate the issue and after debugging, I found that the data was not updating on module installation due to caching. To resolve this, I invalidated the configuration cache tag in the hook_install(), and it is working as expected now. Moving forward for NR. Let me know if anything else needs to be updated regarding this.

Kindly review.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs review » Needs work

Thanks @shalini for working on this issue. However this issue still exists after applying your changes.

Steps to reproduce with your MR applied:

  • On a fresh install enable Project browser module only and verify that 2 sources are available
  • Now enable Project browser devel module and it still shows 2 sources and on configuration page title is still blank
  • Clearing cache manually will fix the source title on config page, but it will still show 2 sources

Using below code in .install can fix it.
\Drupal::service('cache.discovery')->deleteAll();

shalini_jha’s picture

shalini_jha commented
Assigned: Unassigned » shalini_jha

Thanks for your review . i am working on it.

utkarsh_33’s picture

utkarsh_33 commented

@shalini_jha are you still working on this?

shalini_jha’s picture

shalini_jha commented
Assigned: shalini_jha » Unassigned
Status: Needs work » Needs review

Thank you for the review, @narendrar.
While the single tag invalidation seemed to work during my initial testing, I have implemented your suggested change to invalidate the cache.
I have verified the steps again, and this is working as expected.
Please review the updated code and let me know if further changes are required.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs review » Needs work
Issue tags: +Needs tests

This looks good, adding test coverage could be beneficial here.

shalini_jha’s picture

shalini_jha commented
Assigned: Unassigned » shalini_jha

sure, I am checking for test

shalini_jha’s picture

shalini_jha commented
Assigned: shalini_jha » Unassigned
shalini_jha’s picture

shalini_jha commented

The test coverage is not failing in either scenario. After installing the Project Browser Devel module in test coverage , the problem is no longer replicating.it might clear cache in all scenario, I have also attempted to add a functional test in testInstallation(), but it is not showing any failing tests in the same way.
Please Let me know if something is missing for this test, Moving this for NR , to get some help to achieve this test coverage.

shalini_jha’s picture

shalini_jha commented
Status: Needs work » Needs review
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs review » Needs work
Issue tags: -Needs tests

It seems that the cache is already being cleared during tests, which is why this issue is not reproducible. You can proceed with the fix as suggested at https://git.drupalcode.org/project/project_browser/-/merge_requests/621#... and remove the test for now.

If necessary, a follow-up can be created to investigate the test further.

shalini_jha’s picture

shalini_jha commented
Status: Needs work » Needs review

@narendrar Thank you for your suggestion. I have removed the test case as per your feedback and made the necessary adjustments. I have validated that the functionality is working correctly after these changes. I am moving this to 'Needs Review'. Also, I noticed that eslint is failing, but I believe that issue is not related to the changes made here.

narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the reviews, Naren!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.