Problem/Motivation

The Project Browser may become outdated vis a vis the API being provided by Drupal.org. Old clients may not support newer versions of the API. We need to let people know if they need to upgrade Drupal/Project Browser, based on the version of Drupal being used.

Proposed resolution

Display messages provided by Drupal.org that are given in the request from the parent issue.

Endpoint to query: https://www.drupal.org/drupalorg-api/project-browser-filters?drupal_version=11.0.2

CommentFileSizeAuthor
#7 Screenshot 2024-12-28 at 17.15.17.png29.92 KBfjgarlin
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:

Comments

chrisfromredfin created an issue. See original summary.

phenaproxima’s picture

This isn't a stable blocker for Drupal CMS.

fjgarlin’s picture

fjgarlin’s picture

fjgarlin’s picture

Issue summary: View changes

fjgarlin’s picture

Status: Active » Needs review
StatusFileSize
new29.92 KB

This 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:
result

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.

lostcarpark’s picture

Does PB not support D10 any more?

fjgarlin’s picture

PB 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.

fjgarlin’s picture

Bump for review/test on this.

chrisfromredfin’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

So 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.

chrisfromredfin’s picture

(Also, you'll notice I rebased this but it's failing a bunch of tests now?? Was that me? 😬)

fjgarlin’s picture

We 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::VERSION in this case) so I'm not sure which approach to follow.

fjgarlin’s picture

Re tests failing, all nightwatch tests seem to be failing in 2.0.x: https://git.drupalcode.org/project/project_browser/-/jobs/3935754

fjgarlin’s picture

Status: Needs work » Needs review

Adde a test and altered the fixture regeneration to cater for the new parameter. Ready for review again.

narendrar’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Test added, Changes looks good to me. Moving it to RTBC.

chrisfromredfin’s picture

I'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. :)

chrisfromredfin’s picture

Status: Reviewed & tested by the community » Fixed

ship it dot com

fjgarlin’s picture

Agree, 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.

Status: Fixed » Closed (fixed)

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