Closed (fixed)
Project:
Project Browser
Version:
2.0.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Dec 2024 at 15:39 UTC
Updated:
22 Jan 2025 at 15:04 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
phenaproximaThis isn't a stable blocker for Drupal CMS.
Comment #3
fjgarlin commentedComment #4
fjgarlin commentedEndpoint to query: https://www.drupal.org/drupalorg-api/project-browser-filters?drupal_vers...
Comment #5
fjgarlin commentedComment #7
fjgarlin commentedThis is ready for review.
I don't think there are any new errors provoked by these changes. The errors shown in the CI pipeline are not related to this issue:
- CSpell: words in files that are not modified by this MR
- Stylelint: CSS files that are not modified by this MR
- Nightwatch: failures already present in 2.0.x
- PHPUnit: failures already present in 2.0.x
Tested on Drupal 10.3.2:

The message shown comes from https://www.drupal.org/drupalorg-api/project-browser-filters?drupal_vers...
When tested on Drupal 11.0.1 the message does not appear.
Comment #8
lostcarpark commentedDoes PB not support D10 any more?
Comment #9
fjgarlin commentedPB won’t be included as stable and part of core until D11.
In any case, this is not relevant for this issue. This issue is about querying the d.org api endpoint and relaying an error message, if present. Changing the message and/or versions to show the message, would be a drupal.org issue.
Comment #10
fjgarlin commentedBump for review/test on this.
Comment #11
chrisfromredfinSo on principle this looks great to me, however - it really needs a test. I assume a FunctionalJavascript so we can test that the message is displayed on the front-end using JS... but I have no idea how to fake/mock the endpoint result.
But we need to test that if we create a results page with an error, that error is displayed.
I would love to (could be in the future/follow-up) add support for warnings in addition to errors (so we could notify of an upcoming deprecation).
But I think the approach is right.
Comment #12
chrisfromredfin(Also, you'll notice I rebased this but it's failing a bunch of tests now?? Was that me? 😬)
Comment #13
fjgarlin commentedWe won't be able to do a FunctionalJavscript test unless we fake the Drupal version as the query to the endpoint sends the "current" installed Drupal version.
It's not possible to change the value of a constant (
\Drupal::VERSIONin this case) so I'm not sure which approach to follow.Comment #14
fjgarlin commentedRe tests failing, all nightwatch tests seem to be failing in 2.0.x: https://git.drupalcode.org/project/project_browser/-/jobs/3935754
Comment #15
fjgarlin commentedAdde a test and altered the fixture regeneration to cater for the new parameter. Ready for review again.
Comment #16
narendrarTest added, Changes looks good to me. Moving it to RTBC.
Comment #17
chrisfromredfinI'm gonna take this for now because I want it in, but I think the test should have been more generic and/or at a minimum doesn't really belong in the TestDrupalOrgJsonApi - really a test that "if any backend gives an error with its response, that error is displayed" - but this test DOES do that, so I think refactoring in a follow-up is best.
But also, I'm speaking a little bit out of my pay grade, so I welcome any response to this idea. :)
Comment #19
chrisfromredfinship it dot com
Comment #20
fjgarlin commentedAgree, but as this is the very first plugin to implement this it made sense to do it here. We are not sure which conditions could trigger an error on other plugins, but yeah, in any case, a follow-up can address this if needed.