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

[WIP] Manage product's position in multiple categories #1950

Conversation

@vigourouxjulien
Copy link
Contributor

@vigourouxjulien vigourouxjulien commented Jan 29, 2016

Manage product's position in multiple categories.

@gillesbourgeat
Copy link
Member

@gillesbourgeat gillesbourgeat commented Jan 29, 2016

Hello,
The schema.xml and insert.sql file are missing in your PR.
Can you complete the update file 2.3.0-alpha2 not 2.3.0-alpha1
It's good idea to deprecate and create a fallback to the all methods which refer for position in Product Model
Example :

class Product extends BaseProduct implements FileModelParentInterface
{
   ... more code

    /**
     * {@inheritdoc}
     * @deprecated since version 2.3, was moved in the \Thelia\Model\ProductCategory::setPosition
     */
    public function setPosition($v)
    {
        // create a fallback for backward compatibility

        return parent::setPosition($v);
    }

   ... more code
}

This pull request requires a lot of testing
Cheers,

@Asenar
Copy link
Contributor

@Asenar Asenar commented Jan 29, 2016

👍 Thanks you !

This can closes #132 #1941

@Asenar
Copy link
Contributor

@Asenar Asenar commented Feb 24, 2016

I rebased @vigourouxjulien works on current master and done some minor changes in the commit:

  • schema.xml modified / thelia.sql generated
  • used alpha2 instead of alpha1

I wasn't sure about how to do the deprecated thing.

This can be found on https://github.com/asenar/thelia/tree/feature/product_categories_position

@vigourouxjulien vigourouxjulien changed the title Manage product's position in multiple categories [WIP] Manage product's position in multiple categories Mar 3, 2016
@bibich
Copy link
Contributor

@bibich bibich commented Jul 5, 2016

PR #2066 already fixes this issue.
Thanks.

@bibich bibich closed this Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants