Fix label filter not working

[nknguyenhc]

Summary:

Fixes #227

Type of change:

• 🐛 Bug Fix

Changes Made:

Change the value on the model of label-filter-bar component from label.name to label.formattedName.
The value is used to update the filters on issues and PRs.

The error toast shows because on one click, two filters are applied. I believe this is unintentional.
By changing the value to label.formattedName, only one filter is applied (assuming that each label formatted name is unique).

The filter also does not work when the label has 2 parts separated by a period ..

Note that the filter still does not work with labels with 2 or more periods .. This is because of how Labels are constructed. Please see #229 .

Screenshots:

Proposed Commit Message:

Fix label filter not working

Filters takes the value of `label.formattedName` instead of `label.name`.

Checklist:

• I have tested my changes thoroughly.
• I have created tests for any new code files created in this PR or provided a link to a issue/PR that addresses this.
• I have added or modified code comments to improve code readability where necessary.
• I have updated the project's documentation as necessary.

[gycgabriel]

What about the other uses of label.name instead of formattedName, will there be any edge cases from changing only the value here?

[nknguyenhc]

This value being changed to label.formattedName only affects the values in LabelFilterBarComponent::selectedLabelNames. It is only used internally, to update the value of the behaviour subject field LabelFilterBarComponent::selectedLabels. In LabelFilterBarComponent::simulateClick and LabelFilterBar::updateSelection methods:

this.selectedLabels.next(this.selectedLabelNames);

LabelFilterBarComponent::updateSelection value is an @Input field. When looking for places that this component is used, it is only used in FilterBarComponent, in filter-bar.component.html:

<mat-grid-tile class="label-filter-grid-tile" colspan="1">
  <app-label-filter-bar [selectedLabels]="this.labelFilter$" [hiddenLabels]="this.hiddenLabels$"></app-label-filter-bar>
</mat-grid-tile>

Hence any changes to LabelFilterBarComponent::selectedLabels will only cause a change to FilterBarComponent::labelFilter$. Looking at the places that this field is used, it is only used in FilterBarComponent::ngAfterViewInit, which subscribes to changes to this field:

    this.labelFilterSubscription = this.labelFilter$.subscribe((labels) => {
      this.dropdownFilter.labels = labels;
      this.applyDropdownFilter();
    });

On every change, the FilterBarComponent::dropdownFilter.labels is changed, and then applyDropdownFilter is called, where the filter in each of the views$ are updated to FilterBarComponent::dropdownFilter. Here, the views$ are the child components that are filterable. It is an @Input field. Looking at places where FilterBarComponent is used, only in issue-viewer.component.html:

<app-filter-bar [views$]="views" #filterbar></app-filter-bar>

The value of FilterBarComponent::views$ takes the value of IssueViewer::views. IssueViewer::views is a field that follows the value of IssueViewer::cardViews:

  ngAfterViewInit(): void {
    this.viewChange = this.cardViews.changes.subscribe((x) => this.views.next(x));
  }

Which in turn is a @ViewChildren component that is of type QueryList<CardViewComponent>! This means that any change to FilterBarComponent::dropdownFilter will cause a change to the filters of all CardViewComponents within IssueViewer.

TLDR, in short, the change of value from label.name to label.formattedName only means that all CardViewComponents filter by label.formattedName instead of label.name. Note that CardViewComponent has selector app-view-card, so to double check where this change will affect, paste the following code to the browser's console when opening the app:

document.querySelectorAll('app-card-view')

We see that the CardViewComponents are only the columns of the contributors of the current repo. This is the only component that takes the intended effect of the change.

I realised that the LabelFilterBarComponent::hiddenLabelNames needs a similar change. Similar to what I have explained above, the CardViewComponents are the only component that takes the intended effect of the change. However, looks like it only hide the labels. I'm not sure if this is the intended behaviour of the hiding feature, if it not, there needs to be a change to how the CardViewComponents make use of the filter.hiddenLabels.

After hiding the label difficulty.Easy:

[gycgabriel]

However, looks like it only hide the labels. I'm not sure if this is the intended behaviour of the hiding feature, if it not, there needs to be a change to how the CardViewComponents make use of the filter.hiddenLabels.

To confirm, what did you think the hiding feature should do? We might want to reconsider how we present the features or change the feature if it doesn't make intuitive sense to the user

[nknguyenhc]

I think it should hide the issues & PRs that have the tag, instead of simply hide the tag from the page. I have posted a new issue on this at #240.

[gycgabriel]

I'm not sure if WATcher even needs to separate the label names anymore, since unlike CATcher we don't have fixed labels and categories for bug reporting phases etc. Can create another issue to refactor Labels, unless I missed out a use case for WATcher.

Thanks for your detailed explanation

[gycgabriel]

LGTM