AmpersandTarski / Ampersand

Build database applications faster than anyone else, and keep your data pollution free as a bonus.
http://ampersandtarski.github.io/
GNU General Public License v3.0
40 stars 8 forks source link

Boolean population not correctly loaded #1151

Open attaxia opened 3 years ago

attaxia commented 3 years ago

De situatie

Boolean populations are not loaded as specified

Wat er gebeurde er en wat had je verwacht?

A population for a relation with a boolean atom was specified, but the correct values are not loaded in the prototype

Stappen om dit te reproduceren

  1. Load script provided
  2. Generate prototype
  3. Go to "Artikels" interface
  4. Click "artikel1" which is of "artikeltype1"
  5. Click "artikeltype1"
  6. Checkbox is unchecked for "inleverplichtig", although it should be true according to provided population
  7. By clicking the artikeltype1 the checkbox can be checked/unchecked, this value is properly stored and then shown correctly in the Artikel interface

Screenshot / Video

Screen Shot 2021-01-05 at 12 45 05

Context / Source van ampersand script

CONTEXT Loonbewerking IN ENGLISH

CONCEPT Artikel "Een artikel"
CONCEPT Artikeltype "Artikeltype"
CONCEPT Inleverplichtig "Inleverplichtig"
REPRESENT Inleverplichtig TYPE BOOLEAN

RELATION artikelType [Artikel*Artikeltype]              [UNI, TOT]

RELATION inleverPlichtig [Artikeltype*Inleverplichtig]  [UNI, TOT]

POPULATION artikelType CONTAINS
[ ("artikel1", "artikeltype1")
, ("artikel2", "artikeltype2")
, ("artikel3", "artikeltype2")
]

POPULATION inleverPlichtig CONTAINS
[ ("artikeltype1", TRUE)
, ("artikeltype2", FALSE)
]

INTERFACE Artikels : "_SESSION"                                 cRud
BOX <TABS>
     [ "Alle artikels" : V[SESSION*Artikel]                     CRud
       BOX <TABLE>
                [ "Artikel" : I                                 cRud
                ]
     ]

INTERFACE Artikel FOR Artikel : I[Artikel]
BOX [ "Artikel identifier" : I[Artikel]
    , "Artikeltype" : artikelType     
    ]

INTERFACE Artikeltype FOR Artikeltype : I[Artikeltype]
BOX [ "Artikeltype" : I[Artikeltype]
    , "Inleverplichtig" : inleverPlichtig           
    ]

ENDCONTEXT
stefjoosten commented 3 years ago

Analysis (SJ: this analysis is incorrect. See below for the correct analysis.)

I can reproduce this behavior.

Apparently, you expect to edit the relation inleverPlichtig through the interface called Artikel. However, you are asking Ampersand to edit the term artikelType;inleverPlichtig, which has value [ ("artikel1", TRUE), ("artikel2", FALSE) ]. Ampersand blocks you from editing in the term artikelType;inleverPlichtig because this is a composite term (composed of two relations) and Ampersand cannot infer what to insert or delete in which relation. For this reason, you can edit only in primitive terms (i.e. in a relation).

If you want to edit you can use interface Artikeltype, which allows you to edit the relation inleverPlichtig. In your script, this interface is not accessible. So you can make it accessible by adding it to the interface Artikels. Like this:

INTERFACE Artikels : "_SESSION"                                 cRud
BOX <TABS>
     [ "Alle artikels" : V[SESSION*Artikel]                     CRud
       BOX <TABLE>
                [ "Artikel" : I                                 cRud
                ]
     , "Artikeltypes" : V[SESSION*Artikeltype]                  CRud
       BOX <TABLE>
                [ "Artikeltype" : I                             cRud
                ]
     ]

In that case you can navigate to the tab "Artikeltypes" and edit the relation as you please: afbeelding

attaxia commented 3 years ago

@stefjoosten I think you misunderstood the issue. My issue is not that the Artikel interface does not allow editing, this is why I added the Artikeltype interface to begin with.

The issue is that from a clean install the Inleverplichtig checkbox should already be checked for artikeltype1, but it is not. This is based on the supplied population (it should be TRUE).

attaxia commented 3 years ago

I have slightly updated the example code and added a step to the reproduction path to avoid confusion.

stefjoosten commented 3 years ago

Analysis

Let me first establish whether the claim is correct.

When this application starts, the Ampersand database should contain a table for the relation inleverPlichtig that contains pairs "[ ("artikeltype1", TRUE), ("artikeltype2", FALSE) ]. However, what I find is:

afbeelding

This screenshot shows that the relation inleverPlichtig that contains pairs "[ ("artikeltype1", FALSE), ("artikeltype2", FALSE) ]. This proves that @attaxia is right: the initial population is loaded incorrectly in the Ampersand database. (The table "Artikeltype" in the database has an attribute inleverPlichtig. This represents the Ampersand relation inleverPlichtig. 0 in the database represents FALSE in Ampersand.)

stefjoosten commented 3 years ago

@hanjoosten If this has to do with the tech-type BOOLEAN, I would expect this case to be tested in our regression. So in that case, I guess we should extend the regression set. But first we must make a diagnosis.

attaxia commented 3 years ago

Is it correct that this issue still has the wontfix label?

stefjoosten commented 3 years ago

Indeed, that is incorrect. I have removed the wontfix label. It is however not likely that it will be fixed in the short term due to lack of time of all participants. (We could use help...)

stefjoosten commented 3 years ago

@attaxia , By the way, there are other ways to do what you want to do. Ways that make use of relations instead of booleans. Instead of a total, univalent relation inleverPlichtig between artikeltypes and booleans, you can define a relation inleverPlichtig between artikeltypes and the context in which these artikeltypes are inleverplichtig. In this relation you only put the artikeltypes that are inleverplichtig and NOT the ones that are not inleverplichtig. E.g.

RELATION inleverPlichtig [Artikeltype*ONE]

POPULATION inleverPlichtig CONTAINS
[ ("artikeltype1", ONE)
]

I have used ONE as something that defines the context, because there is nothing else in your script I can use for this purpose. In a more elaborate script, this might be (just as an example) an area, a shop, or whatever the context of the inleverplicht might be.

RieksJ commented 3 years ago

There is another way, which is by defining property-relations. Here is what you do:

First, you modify the relation that has a boolean:

--RELATION inleverPlichtig [Artikeltype*Inleverplichtig]  [UNI, TOT]
RELATION inleverPlichtig[Artikeltype] [PROP] -- This is a property relation. '[PROP]' is shorthand for '[SYM,ASY]'. 

All property-relations are subsets of I, so populating them requires the following modification:

POPULATION inleverPlichtig CONTAINS
--[ ("artikeltype1", TRUE)
--, ("artikeltype2", FALSE)
[ ("artikeltype1", "artikeltype1") -- A populated pair is a 'true', an unpopulated pair is 'false'
]

The main difference between the use of property-relations and relations that use booleans is that the latter allow you to determine whether or not the boolean itself is defined. In your example, the construct (I-inleverPlichtig;inleverPlichtig~) tells you that the relation inleverPlichtig is not populated - nobody decided whether or not an Artikeltype is 'inleverplichtig'. You cannot do that with a property-relation: it is simply true or false, and never undefined.

So far, I have only very seldomly come across situations where I needed a BOOLEAN. I prefer property-relations because they are simpler, easier to use in expressions, and render nicely (as a checkbox) in user interfaces.

Michiel-s commented 3 years ago

My analysis of this issue:

  1. When the Ampersand compiler generates the population.json file the initial population is correct

    {
    "links": [
        {
            "src": "artikeltype1",
            "tgt": "True"
        },
        {
            "src": "artikeltype2",
            "tgt": "False"
        }
    ],
    "relation": "inleverPlichtig[Artikeltype*Inleverplichtig]"
    },

    So far so good

  2. When running the prototype installer, the database isn't populated correctly. As @stefjoosten already noticed: image

So here's the problem. The prototype framework doesn't process the boolean value correctly. Let's look into it

  1. Check logs of prototype framework for population initialization

    [2021-02-20 19:18:51] CORE.DEBUG: Add link (artikeltype1[Artikeltype],True[Inleverplichtig]) inleverPlichtig[Artikeltype*Inleverplichtig] to plug [] {"request_id":"aec49b6dc0"}
    [2021-02-20 19:18:51] TRANSACTION.DEBUG: Mark relation 'inleverPlichtig[Artikeltype*Inleverplichtig]' as affected relation [] {"request_id":"aec49b6dc0"}
    [2021-02-20 19:18:51] CORE.DEBUG: Atom artikeltype1[Artikeltype] already exists in concept [] {"request_id":"aec49b6dc0"}
    [2021-02-20 19:18:51] CORE.DEBUG: Atom True[Inleverplichtig] already exists in concept [] {"request_id":"aec49b6dc0"}
    [2021-02-20 19:18:51] DATABASE.DEBUG: UPDATE "Artikeltype" SET "inleverPlichtig" = '0' WHERE "Artikeltype" = 'artikeltype1' [] {"request_id":"aec49b6dc0"}
    [2021-02-20 19:18:51] CORE.INFO: Link added to relation: (artikeltype1[Artikeltype],True[Inleverplichtig]) inleverPlichtig[Artikeltype*Inleverplichtig] [] {"request_id":"aec49b6dc0"}

    Here's the problem. The population (artikeltype1[Artikeltype],True[Inleverplichtig]) is wrongly translated to sql query UPDATE "Artikeltype" SET "inleverPlichtig" = '0' WHERE "Artikeltype" = 'artikeltype1'

  2. So where in the code is this happening? The method Ampersand\Plugs\MysqlDB\MysqlDB::getDBRepresentation in file MysqlDB.php is responsible for this.

  case "BOOLEAN":
    return (int) $atom->getId(); // booleans are stored as tinyint(1) in the database. false = 0, true = 1

The code wrongly casts to integer here, because $atom-getId() return the string "True" and not a boolean True. Casting string "True" to int in php returns 0. Not 1.

  1. Solution The following code replacement will fix it:
    case "BOOLEAN":
    // Booleans are stored as tinyint(1) in the database. False = 0, True = 1
    switch ($atom->getId()) {
        case "True":
            return 1;
        case "False":
            return 0;
        default:
            throw new Exception("Unsupported string boolean value '{$atom->getId()}'. Supported are 'True', 'False'", 500);
    }
    break;
Michiel-s commented 3 years ago

I'm transferring this issue to the Ampersand repo. There we keep track of all issues related to Ampersand compiler and prototype framework. Thanks @attaxia for reporting this issue.

Michiel-s commented 3 years ago

Fixed by commit 48508ad4cd15816539b72f0bd878dac6521c893a

RieksJ commented 3 years ago

While some errors may be fixed in the code, the documentation remains unclear regarding the stuff you can use e.g. in a population, as well as regarding what the template for booleans does. Up till now, I haven't found booleans to be very useful for the projects I work in, which means that I do not know the answers. Nevertheless, it seems worth documenting.

So this issue requests someone that has the answers: