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

Refactor v2 #3

Merged
merged 35 commits into from
Oct 9, 2024
Merged

Refactor v2 #3

merged 35 commits into from
Oct 9, 2024

Conversation

AB-xdev
Copy link
Member

@AB-xdev AB-xdev commented Sep 27, 2024

Fixes #1

@AB-xdev AB-xdev self-assigned this Sep 27, 2024
@AB-xdev AB-xdev force-pushed the refactor-v2 branch 2 times, most recently from 3f248bc to d6354ee Compare October 7, 2024 09:37
@AB-xdev AB-xdev marked this pull request as ready for review October 7, 2024 13:29
@JohannesRabauer
Copy link
Member

Lost features in this refactor:

  1. UI Tests. There are no tests at all. Before migrating to GitHub there were plenty of UI Tests and some Unit-Tests.
  2. Between two dates is now only possible with two conditions.
  3. Greater Or Equals is not possible anymore.
  4. "!=" is not possible anymore.
  5. Conditions are not displayed as badges anymore. That is a visual downgrade.
  6. It is not possible to add predefined filter as a developer, that are not deletable and (possibly) only partly changeable. (see )
  7. It is not possible to limit the comparators anymore. That was possible in v1.
  8. Is it possible to change the locale of the Date-Picker?
  9. Now you have to specify the class of the field:
    .withFilterableField("First Name", Person::firstName, String.class)
    previously that was not needed:
    .withFilter(new SimpleFilterField<>(Person::getFirstName, "First Name"))

Considering these negative changes, i don't get why this would be an upgrade, or a new version. This is clearly a completely different component with roughly the same goal, but a completely different mechanic.
I don't see any reason, why we shouldn't release this as a different component where this v2 is more a SQL-Style Query builder, and v1 is a Simple-Query Builder.

@AB-xdev
Copy link
Member Author

AB-xdev commented Oct 8, 2024

  1. UI Tests. There are no tests at all. Before migrating to GitHub there were plenty of UI Tests and some Unit-Tests.

That is correct. We have no tests anymore as

  • Vaadin Selenium Test require a license key
  • we have no tests in the other components and the template so far -> If we introduce them we should do so in general
  • most test were about configuring the old component. This configuration is no longer present in the new one
  1. Between two dates is now only possible with two conditions.

Removed because

  • there was an external dependency required for that
  • worked only for date-picker fields
  • requires 2 UI fields -> introduces a new components, new value type and so on, which outweights that the same thing can be done with 2 conditions
  1. Greater Or Equals is not possible anymore.

Use > + = if required or create a custom operation.
We can include it if there is enough interest but having too many of drop down options isn't helpful either.

  1. "!=" is not possible anymore.

Use "NOT" + "=".

In general:
Now you can simply add multiple conditions to AND, OR or NOT blocks and these produce the same outcomes.
I removed any combined operations and narrowed them down to the most basic ones. In v1 there was no limit on what operations you could add depending on the use-case, which just bloated the component code and caused a lot of duplications.
Example: Previously there was a IN, NOT IN and CONTAINS operation.
So technically there could have also been a CONTAINS IN and so on. However implementing CONTAINS IN would have been not possible as there were no options for that v1.

If it helps we can create an example inside the demo how to e.g. create a !=, >= or similar condition and add it to the component.

  1. Conditions are not displayed as badges anymore. That is a visual downgrade.

I asked why these were present in the first place and couldn't get an exact reason why - that's why they're missing for now.
It's also kind of hard to implement blocks like AND with them.
We can implement them later if it helps -> Please create a new issue if you still think we should add them.

  1. It is not possible to add predefined filter as a developer, that are not deletable and (possibly) only partly changeable.

You can use the query parameter de/serialization system as a more compact alternative.
However there are currently no Java Object APIs for modifying them as the logic is handled internally.
We can add them if this is required -> new issue.

  1. It is not possible to limit the comparators anymore. That was possible in v1.

This API was replaced by Operations.

  1. Is it possible to change the locale of the Date-Picker?

Yes!
DatePicker is provided inside a supplier, you can change it before it's returned.
(But IMHO default Datepicker is best since it respects the browsers locale if implemented correctly into the system)

  1. Now you have to specify the class of the field: ...

Yes that's the case because it's impossible to predict what column class type the user wants to filter.
Previously there was a constructor for every possible classtype:

public SimpleFilterField(
final FilterProvider.StringProvider<T> provider,
final String description)
{
this.filterField =
BUILDER.withValueProvider(provider, description)
.withNotEqualComparator()
.withEqualComparator()
.withContainsComparator()
.withNotContainsComparator();
}
public SimpleFilterField(
final FilterProvider.EnumProvider<T> provider,
final String description,
final Enum<?>[] enumValues)
{
this.filterField = BUILDER
.withValueProvider(provider, description, enumValues)
.withEqualComparator()
.withNotEqualComparator()
.withContainsComparator()
.withNotContainsComparator();
}
public SimpleFilterField(
final FilterProvider.NumberProvider<T> provider,
final String description)
{
this.filterField = BUILDER
.withValueProvider(provider, description)
.withEqualComparator()
.withNotEqualComparator()
.withContainsComparator()
.withNotContainsComparator()
.withLessThanComparator()
.withLessThanOrEqualsComparator()
.withGreaterThanComparator()
.withGreaterThanOrEqualsComparator();
}
public SimpleFilterField(
final FilterProvider.LocalDateProvider<T> provider,
final String description)
{
this.filterField = BUILDER
.withValueProvider(provider, description)
.withIsBeforeComparator()
.withIsBeforeOrEqualsComparator()
.withIsAfterComparator()
.withIsAfterOrEqualsComparator()
.withIsBetweenComparator();
}
public SimpleFilterField(
final FilterProvider.LocalDateTimeProvider<T> provider,
final String description)
{
this.filterField = BUILDER
.withValueProvider(provider, description)
.withIsBeforeComparator()
.withIsBeforeOrEqualsComparator()
.withIsAfterComparator()
.withIsAfterOrEqualsComparator();
}
public SimpleFilterField(
final FilterProvider.BooleanProvider<T> provider,
final String description)
{
this.filterField = BUILDER
.withValueProvider(provider, description)
.withEqualComparator()
.withNotEqualComparator();
}

public <T> FilterField<T, String> withValueProvider(
final FilterProvider.StringProvider<T> provider,
final String description)
{
return this.withValueProvider(provider, description, String.class);
}
public <T> FilterField<T, Number> withValueProvider(
final FilterProvider.NumberProvider<T> provider,
final String description)
{
return this.withValueProvider(provider, description, Number.class);
}
public <T> FilterField<T, LocalDate> withValueProvider(
final FilterProvider.LocalDateProvider<T> provider,
final String description)
{
return this.withValueProvider(provider, description, LocalDate.class);
}
public <T> FilterField<T, LocalDateTime> withValueProvider(
final FilterProvider.LocalDateTimeProvider<T> provider,
final String description)
{
return this.withValueProvider(provider, description, LocalDateTime.class);
}
public <T> FilterField<T, Boolean> withValueProvider(
final FilterProvider.BooleanProvider<T> provider,
final String description)
{
return this.withValueProvider(provider, description, Boolean.class);
}
public <T> FilterField<T, Enum> withValueProvider(
final FilterProvider.EnumProvider<T> provider,
final String description,
final Enum<?>[] enumValues)
{
return new FilterFieldEnumExtension<>(provider, description, Enum.class, new ArrayList<>(), enumValues);
}
public <T, X> FilterField<T, X> withValueProvider(
final ValueProvider<T, X> provider,
final String description,
final Class<X> type)
{
Objects.requireNonNull(provider);
Objects.requireNonNull(description);
return new FilterField<>(provider, description, type, new ArrayList<>());
}

Considering these negative changes, i don't get why this would be an upgrade

Let's have a talk/call about this topic tomorrow where I can explain these points in more detail without writing half a novel before Feierabend ^^

@AB-xdev
Copy link
Member Author

AB-xdev commented Oct 9, 2024

Conclusion after discussion:

  • We will try to improve our communications in both directions
  • v1 will be split into a separate repository and developed there

Copy link

sonarqubecloud bot commented Oct 9, 2024

@AB-xdev AB-xdev removed the request for review from JohannesRabauer October 9, 2024 07:39
@AB-xdev AB-xdev merged commit d58b765 into develop Oct 9, 2024
7 checks passed
@AB-xdev AB-xdev deleted the refactor-v2 branch October 9, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v2 Redesign
2 participants