Problem/Motivation

The maintenance/development status/security coverage filters all appear in the box, but module categories do not
Nor does "clear filters" remove any selected module categories

Steps to reproduce

Proposed resolution

Remaining tasks

  • ✅ File an issue about this project
  • ☐ Addition/Change/Update/Fix to this project
  • ☐ Testing to ensure no regression
  • ☐ Automated unit/functional testing coverage
  • ☐ Developer Documentation support on feature change/addition
  • ☐ User Guide Documentation support on feature change/addition
  • ☐ Code review from 1 Drupal core team member
  • ☐ Full testing and approval
  • ☐ Credit contributors
  • ☐ Review with the product owner
  • ☐ Release

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork project_browser-3281620

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett created an issue. See original summary.

srishtiiee’s picture

srishtiiee CreditAttribution: srishtiiee at Acquia commented
Assigned: Unassigned » srishtiiee

srishtiiee opened merge request !106

srishtiiee’s picture

srishtiiee CreditAttribution: srishtiiee at Acquia commented
Assigned: srishtiiee » Unassigned
Status: Active » Needs review
Issue tags: -Needs tests
narendraR’s picture

narendraR
He/Him
Primary language English
Location Jaipur, India
CreditAttribution: narendraR as a volunteer and at Acquia commented

Tested manually and functionality is working fine.

narendraR’s picture

narendraR
He/Him
Primary language English
Location Jaipur, India
CreditAttribution: narendraR as a volunteer and at Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Reviewed & tested by the community » Needs work

Checked the code and we do have now two calls to the same endpoint. We should have only one and then pass data as needed between components/stores, etc.

srishtiiee’s picture

srishtiiee CreditAttribution: srishtiiee at Acquia commented
Status: Needs work » Needs review
fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Assigned: Unassigned » fjgarlin
fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Assigned: fjgarlin » Unassigned
Status: Needs review » Reviewed & tested by the community

Looks good to me. Categories now appear in the selected filters and are cleared with the "Clear filters" link and the code uses only one call for the categories and then pass that information between components through the store. Marking as RTBC. Thanks!!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Reviewed & tested by the community » Needs work

Two points of feedback.

srishtiiee’s picture

srishtiiee CreditAttribution: srishtiiee at Acquia commented
Status: Needs work » Needs review

Made the required changes to the test fixture and corrected the comment. Thanks for noticing it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

Thanks for the fixes, if this were real data that'd be enough. But right now we hardcode the project_usage_total value instead of summing up the usage (not sure why!) so we need that value changed as well (or switch to computing that in the install?)

Let's just hardcode the numbers in both places in this issue, and open a followup for computing it.

srishtiiee’s picture

srishtiiee CreditAttribution: srishtiiee at Acquia commented

Whoops! Sorry for not being attentive enough. It was hard to see with the tests passing both locally and here.🙈
Created a followup issue- #3282201

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Needs review » Reviewed & tested by the community

Since this was RTBC before and the needed changes were minor, I'm going to RTBC this and then merge shortly. Thanks @srishtiiee!

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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