If pk not in request then use ParamConverter name #190

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
@redexp

redexp commented Nov 7, 2012

It's useful when using FOSRestBundle with automatic routing loader and ParamConverter.

    /**
     * @View()
     * @ParamConverter("user", options={"mapping": {"user": "id"}})
     */
    public function getUserAction(Model\User $user)
    {
        return $user;
    }

you know FOSRestBundle will generate for this method route like

/users/{user}

Without that ParamConverter propel can't figure out which parameter is pk
With my changes you don't need to map "user" parameter to "id"

@willdurand

This comment has been minimized.

Show comment
Hide comment
@willdurand

willdurand Nov 8, 2012

Member

ping @jaugustin

William Durand | http://www.williamdurand.fr

On Wed, Nov 7, 2012 at 6:01 PM, Sergii Kliuchnyk
notifications@github.comwrote:

It's useful when using FOSRestBundle with automatic routing loader and
ParamConverter.
With my changes you don't need to map "user" parameter to "id" like here

/**     * @View()     * @ParamConverter("user", options={"mapping": {"user": "id"}})     */
public function getUserAction(Model\User $user)
{
    return $user;
}

Without that ParamConverter can't figure out which parameter is pk

You can merge this Pull Request by running:

git pull https://github.com/redexp/PropelBundle 1.1

Or view, comment on, or merge it at:

#190
Commit Summary

  • If pk not in request then take first param for it
  • If pk not in request then use ParamConverter name

File Changes

  • M Request/ParamConverter/PropelParamConverter.php (4)

Patch Links

Member

willdurand commented Nov 8, 2012

ping @jaugustin

William Durand | http://www.williamdurand.fr

On Wed, Nov 7, 2012 at 6:01 PM, Sergii Kliuchnyk
notifications@github.comwrote:

It's useful when using FOSRestBundle with automatic routing loader and
ParamConverter.
With my changes you don't need to map "user" parameter to "id" like here

/**     * @View()     * @ParamConverter("user", options={"mapping": {"user": "id"}})     */
public function getUserAction(Model\User $user)
{
    return $user;
}

Without that ParamConverter can't figure out which parameter is pk

You can merge this Pull Request by running:

git pull https://github.com/redexp/PropelBundle 1.1

Or view, comment on, or merge it at:

#190
Commit Summary

  • If pk not in request then take first param for it
  • If pk not in request then use ParamConverter name

File Changes

  • M Request/ParamConverter/PropelParamConverter.php (4)

Patch Links

@jaugustin

This comment has been minimized.

Show comment
Hide comment
@jaugustin

jaugustin Nov 8, 2012

Member

I am not in favor of this hack, because it will break everything

this PR totaly by pass the mapping informations and break the use of smart query by column name,

Member

jaugustin commented Nov 8, 2012

I am not in favor of this hack, because it will break everything

this PR totaly by pass the mapping informations and break the use of smart query by column name,

@redexp

This comment has been minimized.

Show comment
Hide comment
@redexp

redexp Nov 8, 2012

Why it will break everything? As for me it's very logical If argument is Propel model then pk for it should be or in parameter which name equals to pk field of this model or in parameter which name is equal to name of this argument

redexp commented Nov 8, 2012

Why it will break everything? As for me it's very logical If argument is Propel model then pk for it should be or in parameter which name equals to pk field of this model or in parameter which name is equal to name of this argument

@jaugustin

This comment has been minimized.

Show comment
Hide comment
@jaugustin

jaugustin Nov 8, 2012

Member

Did you try to run tests?

You always force the query by PK, even if it's not wanted by the user

you could create your own paramConverter to do this but this will not be integrated like this in PropelBundle

Member

jaugustin commented Nov 8, 2012

Did you try to run tests?

You always force the query by PK, even if it's not wanted by the user

you could create your own paramConverter to do this but this will not be integrated like this in PropelBundle

@redexp redexp closed this Nov 8, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment