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

[Review] SyliusReviewBundle #3980

Merged
merged 30 commits into from
Feb 19, 2016
Merged

Conversation

GSadee
Copy link
Member

@GSadee GSadee commented Jan 26, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR

Initial implementation of SyliusReviewBundle, which allows to easily manage review system. It's based on @Zales0123 #3300.

"Reviewable" and "Reviewer" classes can be configured under sylius_review tag:

sylius_review:
    resources:
        product:
            subject: %sylius.model.product.class%
            review:
                classes:
                    model: Sylius\Component\Core\Model\ProductReview
                    form:
                        default: Sylius\Bundle\CoreBundle\Form\Type\ProductReviewType
                        admin: Sylius\Bundle\CoreBundle\Form\Type\ProductReviewAdminType
            reviewer:
                classes:
                    model: Sylius\Component\Core\Model\Customer

I have added changes from #3372 and extended this PR to use CustomerGuestType in ReviewType.

// this up() migration is auto-generated, please modify it to your needs
$this->abortIf($this->connection->getDatabasePlatform()->getName() != 'mysql', 'Migration can only be executed safely on \'mysql\'.');

$this->addSql('ALTER TABLE sylius_review DROP FOREIGN KEY FK_43626F3A4584665A');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for these (3) migrations! Only 1 is sufficient, which adds/removes review table :D

@pjedrzejewski
Copy link
Member

@GSadee Rebase has gone wrong this one probably... :)

@nakashu
Copy link
Contributor

nakashu commented Feb 2, 2016

@GSadee @pjedrzejewski can you estimate when this will be finished? Got a project in development where reviews are needed, so wanted to know if I should wait or implement it on my own.
thanks.

@pjedrzejewski
Copy link
Member

We need this too and we want to get it merged by the end of the week. I would review it today already but rebase went really wrong...

@michalmarcinkowski
Copy link
Contributor

@nakashu it will be done ASAP. We want to merge it this week.

@nakashu
Copy link
Contributor

nakashu commented Feb 2, 2016

great to hear. thanks

@nakashu
Copy link
Contributor

nakashu commented Feb 2, 2016

I'll probably add support for schema aggregate rating in PR after you merge this. (if it is not present.)

Propabably for another RFC, is how to implement it.

  • json-lc - this can be decoupled from temlplate, but is less supported/trusted
  • microdata - coupled to templates, or using macros and some crazy template stuff.

https://schema.org/AggregateRating

WIll create an RFC after merge.

Scenario: Seeing created reviews in the list
Given I am on the dashboard page
When I follow "Product reviews"
Then I should see 3 reviews in the list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shall be consistent and align all new scenarios to left.

},
"extra": {
"branch-alias": {
"dev-master": "0.17/-dev"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove /.

pjedrzejewski pushed a commit that referenced this pull request Feb 19, 2016
@pjedrzejewski pjedrzejewski merged commit 8ef740e into Sylius:master Feb 19, 2016
@pjedrzejewski
Copy link
Member

Long road but we have made it, thanks Grzegorz, Mateusz, Daniel and everyone involved! 👍 🎆

@michalmarcinkowski
Copy link
Contributor

That was a big one! Great to see it merged! Thanks everyone 👍

@Zales0123
Copy link
Member

I'm really glad to see it merged 🎉

@GSadee GSadee deleted the sylius-review-bundle branch September 22, 2017 08:31
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet