Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

eq doesn't work #1823

Closed
brianchu opened this issue Sep 21, 2022 · 15 comments
Closed

eq doesn't work #1823

brianchu opened this issue Sep 21, 2022 · 15 comments
Labels
android This issue directly affects structure and logic of Android project bug Something isn't working epic Global features request help wanted iOS This issue directly affects structure and logic of iOS project web This issue directly affects structure and logic of Web project

Comments

@brianchu
Copy link

Please provide all the information requested. Issues that do not follow this format are likely to stall.

Description

Please provide a clear and concise description of what the bug is. Include screenshots if needed.
Please test using the latest Beagle release to make sure your issue has not already been fixed: https://github.com/ZupIT/beagle/releases

Steps To Reproduce

Provide a detailed list of steps that reproduce the issue.

  1. Launch the playground
  2. Run conditional-action.json with modification: replace gte function with eq function
    ...
"onPress": [
        {
          "_beagleAction_": "beagle:condition",
          "condition": "@{eq(user.age, 18)}",
          "onTrue": [
            {

...

Expected Results

Setting the age to 18 will result buying a beer.

What actually happen: never able to buy a beer.

@brianchu brianchu added the bug Something isn't working label Sep 21, 2022
@Tiagoperes
Copy link
Contributor

Tiagoperes commented Sep 23, 2022

Hi @brianchu, hope you're ok!

I figured you're referring to this example: https://playground.usebeagle.io/#/demo/component-interaction/condional-action.json?platform=ios

I checked and there's a bug with Beagle Android and Beagle iOS. The comparison operations should make automatic type coercions. It gives you false because the string "18" is not equal to the integer 18. It is a string because the value comes from a text field.

We'll fix this to make every comparison function auto coerce Strings to Int or Doubles if they represent these types, but while this is not fixed, you can use a custom operation as a workaround.

You can either create a custom operation eq with type coercion or implement an operation that converts string to Int.

Example of the second: eq(int(user.age), 18).

Documentation for custom operations: https://docs.usebeagle.io/v2.0/api/operations/how-to-register-a-new-operation/

@Tiagoperes Tiagoperes added android This issue directly affects structure and logic of Android project iOS This issue directly affects structure and logic of iOS project labels Sep 23, 2022
@Tiagoperes
Copy link
Contributor

Tiagoperes commented Sep 23, 2022

Added to the backlog:

1. Make the following operations agnostic to the type of the data: eq, gt, lt, gte, lte.

  • If one is a string and the other is an integer, parse the string to integer using the decimal base and then compare. If there's a parse error, return false.
  • If one is a string and the other is double, parse the string to double using the decimal base and . as the decimal part separator. If there's a parse error, return false.
  • If one is an integer and the other is a double, check if the decimal part of the double is 0, if it is, compare the integer part. Otherwise, return false.

2. Make the following operations agnostic to the type of the data: sum, subtract, multiply, divide.

  • The result type should be: a double if the operation is divide; a double if at least one of the arguments is a double or a string representing a double; an integer otherwise.
  • If the result type is double and an argument is an integer, the argument should be converted to double.
  • If the result type is double and an argument is a string, the argument should be parsed to double using the decimal system and .as the decimal separator.
  • If the result type is integer and an argument is a string, the argument should be parsed to integer using the decimal system.

3. Create operations to convert types:

  • int(String): parses the String into an integer (decimal). On parse error return NaN, if supported, or null otherwise.
  • int(Double): converts the Double into an Integer by truncating its decimal part.
  • double(String): parses the String into a double (decimal, . as the separator). On parse error return NaN, if supported, or null otherwise.
  • double(Int): converts the Integer into a Double.
  • string(Int): creates a string representation of the integer.
  • string(Double): creates a string representation of the double (. as the separator).

Platforms

1 and 2 should be implemented on Android and iOS. 3 should also be implemented on web platforms.

@Tiagoperes Tiagoperes added web This issue directly affects structure and logic of Web project epic Global features request labels Sep 23, 2022
@TomerPacific
Copy link

Hi @Tiagoperes , would you like help with this issue?

@Tiagoperes
Copy link
Contributor

Tiagoperes commented Oct 6, 2022

Hi @TomerPacific, sorry for the delay. We already implemented this for iOS, (see the PR above). We would start it for Android next week. We'd much appreciate the help, let me know if you're going to implement this so I can adjust our backlog.

If you decide to help us with a PR on Beagle Android, take a look at the iOS implementation, a good idea is to copy all test cases and just translate them to Kotlin.

On Web we just need to implement the new operations (3). We intend to do this soon, but any help would be appreciated.

@TomerPacific
Copy link

TomerPacific commented Oct 20, 2022

@Tiagoperes - Sure. I'll try to do my best.

Just to make sure I am on the right path, I have opened a DRAFT PR so you can view the changes I making.

Let me know if I am good to proceed or if I am totally on the wrong path.

@Tiagoperes
Copy link
Contributor

@TomerPacific tyvm for your contribution. I'll ask @hernandazevedozup to take a look and get back to you asap!

@TomerPacific
Copy link

@Tiagoperes - waiting for @hernandazevedozup's feedback to continue with this.

@hernandazevedozup
Copy link
Contributor

@TomerPacific Thank you very much for the contribution, the feedback is here
ZupIT/beagle-android#106 (comment)

@TomerPacific
Copy link

@hernandazevedozup , I've done some more work:

  • I removed the duplication of the method handleOperation
  • I added a TypeConverter class for item 3
  • I refactored OperationSum and OperationDivide

I am unsure if my implementation for these two operations was correct as the details in item 2 are a bit confusing.

Do you mind looking into what I have done and letting me know if it's correct?
I would appreciate your feedback.
Thanks

@hernandazevedozup
Copy link
Contributor

hernandazevedozup commented Oct 26, 2022

@hernandazevedozup , I've done some more work:

  • I removed the duplication of the method handleOperation
  • I added a TypeConverter class for item 3
  • I refactored OperationSum and OperationDivide

I am unsure if my implementation for these two operations was correct as the details in item 2 are a bit confusing.

Do you mind looking into what I have done and letting me know if it's correct? I would appreciate your feedback. Thanks

@TomerPacific Thanks for you help, for the item 2 I believe your implementation could aim to support all the scenarios written by the swift implementation unit tests that can be found here ZupIT/beagle-ios#47

@TomerPacific
Copy link

@hernandazevedozup - I looked over those tests before my question. I am still a bit confused regarding some of the phrasing in item number 2.
For example,

The result type should be: a double if the operation is divide; a double if at least one of the arguments is a double or a string representing a double; an integer otherwise.

Let's say we are talking about the sum operation. If one of the arguments in the list of numbers is a double, then the result should be a double. But then you have line number 2 stating:

If the result type is double and an argument is an integer, the argument should be converted to double.

Then, what does it mean? That I need to convert all the list of numbers in the list to doubles? Meaning, if there is one double in a list of sum, everything needs to be converted to a double?

@Tiagoperes
Copy link
Contributor

Tiagoperes commented Oct 26, 2022

@TomerPacific This will depend on the language you're using. If the language can sum Int and Double and give you a Double result, then you don't need to do anything else, the language itself made these conversions under the hood for you.

In the case of Kotlin, you can ignore this statement (line 2).

@hernandazevedozup
Copy link
Contributor

hernandazevedozup commented Oct 27, 2022

@TomerPacific This will depend on the language you're using. If the language can sum Int and Double and give you a Double result, then you don't need to do anything else, the language itself made these conversions under the hood for you.

In the case of Kotlin, you can ignore this statement (line 2).

@Tiagoperes As you said, in kotlin what happens is this example below:
In kotlin, the conversion is made automatically for the following example(double/int = double) ->>> 6.0 / 2 = 3.0

@TomerPacific
Copy link

TomerPacific commented Nov 4, 2022

@Tiagoperes , @hernandazevedozup - updating that I am still working on the feature. It may take a long time. If you prefer to pass it to someone else, then do so.

@Tiagoperes
Copy link
Contributor

Hi @TomerPacific. We really appreciate your help, but we need this feature asap. For this reason, @hernandazevedozup will pick this up from where you left it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
android This issue directly affects structure and logic of Android project bug Something isn't working epic Global features request help wanted iOS This issue directly affects structure and logic of iOS project web This issue directly affects structure and logic of Web project
Projects
None yet
Development

No branches or pull requests

5 participants