Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unique inversable flag #8

Closed
PowerKiKi opened this issue Jun 22, 2018 · 5 comments
Closed

Unique inversable flag #8

PowerKiKi opened this issue Jun 22, 2018 · 5 comments
Assignees
Labels
enhancement New feature or request natural-search

Comments

@PowerKiKi
Copy link
Member

A flag value must always be unique, meaning it can only be selected exactly 1 time per group. Because having a flag multiple times make no sense, because the value is always the same.

Also a flag must be configurable as "inversed", meaning that the flag value is set when the flag is not selected, and the flag value is removed when the flag is selected. This is to express "include archive" flag, which by default filter out archived items, without having to be visible in the search bar. Once the "include archive" is selected, it appears in the search bar and it stops filtering out archived items. So the value is not present anymore.

@sambaptista do you agree with the inversable thing ?

@PowerKiKi PowerKiKi self-assigned this Jul 3, 2018
@sambaptista
Copy link
Member

sambaptista commented Jul 3, 2018

I think you can remove the "inversable" from your title or maybe replace it by an "overridable".

We can solve this use case with a previous conversation we had about contexts. Consider reading this issue first #3

We need a generic system to mark a configuration as unique

  • It may be implicit true for flag typed configurations
  • It would be explicit for some components like "Multiple select"

Let's ignore the visual aspect here because it depends on #3

We have two use cases :

  • Allow a user to add a filter and not be able to add it anymore
  • Allow a user to override a context (see contextualisation Preselections #2)

The unique attribute

A "unique" attribute is setted at configuration level #3 and accepts a boolean. Unique configurations appear in the config selection menu only if there is not yet an input with same field name and unique attribute or if there is one but it's a context selection.

Following this idea, an unique configuration can be added once by selection type : context and user selection. For real, it can therefore be added twice.

Solve your use case
We have two solutions to solve your use case.

The basic idea is to contextualise natural-search with an invisible selection "without archives" and then allow the user override that selection on demand.

For that, we need to provide natural-search with a a context and two opposite flag configurations.

The context is provided by the parent component and force to filter items without archives :

const selections = [
[{
      "field": "archived",  // name must be the same on context and configs
      "visible": false, // Prevents user to see it. We could use force: true #3
      "condition": {
        "equal": {
          "value": "true"
        }
      }
}]
],

Then, there are the two flag configurations :

const configurations = [
  // With archives : is visible for end user
  {
    "display": "With archives",
    "field": "archived", // name must be the same on context and configs
    "condition": {
      "equal": {
        "value": true
      }
    }
  },

  // Without archives is required for `context` to be accepted, but is hidden to prevent user to use it.
  {
    "display": "Without archives",
    "field": "archived",  // name must be the same on context and configs
    "visible": false,
    "condition": {
      "equal": {
        "value": false
      }
    }
  }
]

@PowerKiKi
Copy link
Member Author

OK for me except:

Following this idea, an unique configuration can be added once by selection type : context and user selection. For real, it can therefore be added twice.

Why would we want to have the same, identical value appear twice in the final output ? I don't think we should make a distinction between context or not context when we check for uniqueness.

@PowerKiKi PowerKiKi removed their assignment Jul 16, 2018
@PowerKiKi PowerKiKi transferred this issue from Ecodev/natural-search Apr 21, 2019
@PowerKiKi PowerKiKi added enhancement New feature or request natural-search labels Apr 21, 2019
@PowerKiKi
Copy link
Member Author

Re-reading everything, I am not so sure about the overriding thing. The usage seems way too brittle, because you must configure two things in two (very) different places in your project and you must have a matching name, otherwise nothing work. I can see easily how we will forget about that in future maintenance and breaks the app in a non-obvious way.

Also this would make it blocked by #2.

Having a single boolean on FlagFacet like so:

/**
 * Facet that is only a flag (set or unset)
 */
export interface FlagFacet extends BasicFacet {

    /**
     * The value to be returned when the flag is set
     */
    condition: any;

    /**
     * Whether the value is set when the flag exist or does not exist
     */
    inversed: boolean;
}

makes it not only much easier to implement in this lib, but also much easier to use and maintain in projects.

@sambaptista
Copy link
Member

The reason I've argued about overriding, is that selection.visible and facet.visible are nice to have features and they could be done out of the usecase of this issue.

The overriding system, combined with visible attributes would be generic and powerfull enough to satisfy the usecase of this issue.

Adding facet.inversed attribute is an additional and specific feature for this usecase that works more like a shortcut. For sure it should remove developper errors... so, let's say I agree.

With overriding system the preselections #2 block this ticket. Without preselections #2 it's not blocked.

@PowerKiKi PowerKiKi self-assigned this Aug 24, 2019
@PowerKiKi
Copy link
Member Author

Unique constraint was not implemented because of lack of time, but it could/should be added later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request natural-search
Projects
None yet
Development

No branches or pull requests

2 participants