propelorm / Propel

Current stable (and outdated and unmaintained) version of Propel - Please use v2 https://github.com/propelorm/Propel2 --
http://www.propelorm.org
MIT License
841 stars 417 forks source link

multiple aggregate_column behaviors not working #731

Open auss opened 11 years ago

auss commented 11 years ago

When I use aggregate_column more than once, only the last one has generated code. In this example

<table name="shopping_cart">
        <column name="id"                           type="INTEGER" primaryKey="true" autoIncrement="true" required="true"/>
        <column name="session_id"                   type="VARCHAR" required="true" size="255" />
        <column name="status"                       type="INTEGER" required="true" default="0" />
        <column name="client_id"                    type="INTEGER" required="false" />
        <column name="total_price"                  type="DECIMAL" required="false" size="12" scale="2" />
        <column name="item_count"                   type="INTEGER" required="false" default="0" />

        <behavior name="aggregate_column">
            <parameter name="name" value="total_price" />
            <parameter name="foreign_table" value="shopping_cart_item" />
            <parameter name="expression" value="SUM( net_price * quantity )" />
        </behavior>

        <behavior name="aggregate_column">
            <parameter name="name" value="item_count" />
            <parameter name="foreign_table" value="shopping_cart_item" />
            <parameter name="expression" value="SUM( quantity )" />
        </behavior>

        <behavior name="timestampable" />
        <behavior name="multi_domain">
            <parameter name="copy_content" value="false" />
        </behavior>

        <foreign-key foreignTable="sessions">
            <reference local="session_id" foreign="session_id"/>
        </foreign-key>

        <unique>
            <unique-column name="session_id"></unique-column>
        </unique>

    </table>

propel generated only funcitons for last declaration:

// aggregate_column behavior

    /**
     * Computes the value of the aggregate column item_count *
     * @param PropelPDO $con A connection object
     *
     * @return mixed The scalar result from the aggregate query
     */
    public function computeItemCount(PropelPDO $con)
    {
        $stmt = $con->prepare('SELECT SUM( quantity ) FROM "shopping_cart_item" WHERE shopping_cart_item.shopping_cart_id = :p1');
        $stmt->bindValue(':p1', $this->getId());
        $stmt->execute();

        return $stmt->fetchColumn();
    }

    /**
     * Updates the aggregate column item_count *
     * @param PropelPDO $con A connection object
     */
    public function updateItemCount(PropelPDO $con)
    {
        $this->setItemCount($this->computeItemCount($con));
        $this->save($con);
    }

    // timestampable behavior
auss commented 11 years ago

ping @propelorm

ghost commented 11 years ago

I think it's related to the way behaviors are added to tables: each table have an array of behaviors where the key is the name of the behavior. Now: when you use different behavior statements with the same name attribute, the first one is overridden by last one.

I think it can be fixed easily overriding setupObject() from Behavior.php where the name is set, maybe with the column name concatenated to 'aggregate_column'. In my opinion it's the fastest way to fix this issue but I don't know if it will break something. By the way in documentation it's pointed out that you can use this behavior many times on the same table.

What do you think, @willdurand ?

willdurand commented 11 years ago

@andrea-chris you are right.. I thought it was fixed but.. obviously it is not...

ghost commented 11 years ago

Question: is it possible to add an optional "id" parameter to "behavior" tag?

So Propel will use "name" parameter to load behavior class which will use "id" parameter to build its own name if it exists. I think it's the better way to solve this issue and to allow developers to write their own multi-behavior.

Do I miss something?

peter17 commented 8 years ago

Comment #222 #issuecomment-62825257 says it works, but with Propel-1.7.1, it doesn't...