Audit the unused Project properties

I reviewed the properties marked as unused in the Project file. I found that 'selector_id' is being used but no others are referenced from any svelte files.

I removed some parameters from the public function, since they are not needed in Svelte. Although, I left the private functions to set those parameters, since they are used when getting the project data from drupal.org, and we'll probably want to have that in case there is a need to display that info.

The Star User Count is never used and is only ever set to a static value. So this could likely be removed completely.

This looks good and tests are passing. However, I'd like Tim or Fran to give it a review first.

I think that if we are going to remove the exposed properties because they are not used, then we should also remove all the associated methods, like setters and getters.

For example: https://git.drupalcode.org/search?search=setIsActive&nav_source=navbar&p...

chrisfromredfin → closed merge request !386

chrisfromredfin → changed the visibility of the branch 3309273-audit-the-unused to hidden.

I've refactored this since the previous one was so dated, but I think this is better and spreads deeper into the codebase; would love Fran to look at it.

I left some feedback in the MR. If we are cleaning up purely based on what's shown in the svelte templates, then it might be ok, but there are certain properties that are in use one way or another (ie: via filters) or that should be used but might be not (ie: URL).

Rebased this to the latest 2.0.x.There are a couple of issues that @fjgarlin pointed related to isMaintained and URL so wanted a consensus on what needs to be done.I'll again mark this NR just to double confirm what are next steps on this and maybe ping in the slack channel .

We definitely want to keep URL - we're using that on the detail page now. What I'm amazed by is that getting rid of isMaintained here still lets the "maintained" filter work on the front-end. Why is that?! Do we not need that in the backend object because the front-end doesn't directly use the backend object? Or does the JSON:API endpoint just ignore filters provided that don't matter?

I can't seem to find an example where an unmaintained module shows with manual testing. Can anyone help?

The first order of business here is for someone to digest and understand the interplay between the unused project properties we're removing from the PHP objects, and how those are then used on the frontend. Me, without that knowledge, cannot review this and know if getting rid of these properties "works."

But I can say we need isMaintained and URL on the front-end, for sure.

I see field_maintenace_status here: https://www.drupal.org/jsonapi/index/project_modules?filter%5Bmachine_na... →
and a query using the filter here →

    // 'Actively maintained'.
    '49125cb6-2f35-451b-922d-3042cb1b4391',
    // 'Minimally maintained'
    'de457d5a-2ce3-45b4-b88b-1c54e5e6d0e2',
    // 'Seeking co-maintainer(s)
    'fd8b539f-a5e4-4577-9367-f119a252327b',

isMaintained is just a boolean that says whether the maintenance status of a module is within those 3 posted above.
Code: isMaintained: in_array($maintenance_status['id'], self::MAINTAINED_VALUES),

The filters work independently from how that property is calculated.

I don't see a problem with this.

RTBC+1. The cleanup looks good.

chrisfromredfin → changed the visibility of the branch 3309273-audit-unused-v2 to hidden.

This test seems to fail consistently with the removal of these properties. This needs to be investigated further.

Note that in issue 📌 Do not hardcode UUIDs in DrupalJsonApi plugin Active , we are changing some of the properties that will be removed in this issue, only to avoid a hard dependency on this issue.

If 📌 Do not hardcode UUIDs in DrupalJsonApi plugin Active , we will need to clean here 3 extra lines that were added. If this one gets merged first, I'll take care of things on the other issue.

The lines to remove are in DrupalDotOrgJsonApi.php file and are:

    // @todo Remove the next three lines when https://www.drupal.org/project/project_browser/issues/3309273 is merged.
    $filter_values = $this->filterValues();
    $active_values = $filter_values['active'] ?? [];
    $maintained_values = $filter_values['maintained'] ?? [];

Then, when rebasing, you might get a conflict in the following lines:

          isMaintained: in_array($maintenance_status['id'], $maintained_values),
          ...
          isActive: in_array($development_status['id'], $active_values),

All you need to do to fix the conflict, is remove those lines entirely, as that is what this issue is intending to do.

At first, with this MR applied, when I go to a module detail page, I get 5 warnings - one for each removed property.

Deprecated function: Creation of dynamic property Drupal\project_browser\ProjectBrowser\Project::$created is deprecated in Drupal\Component\Serialization\PhpSerialize::decode() (line 21 of core/lib/Drupal/Component/Serialization/PhpSerialize.php).

However, I did a fresh install of Drupal (drush si) and it wasn't there - so I'm guessing that was from lingering cache / KeyValueStore stuff from testing other MR's. I think this is good, especially with passing tests!