Skip to content

[WIP] [Review] SyliusReviewBundle#3300

Closed
Zales0123 wants to merge 16 commits intoSylius:masterfrom
Zales0123:sylius-review-bundle
Closed

[WIP] [Review] SyliusReviewBundle#3300
Zales0123 wants to merge 16 commits intoSylius:masterfrom
Zales0123:sylius-review-bundle

Conversation

@Zales0123
Copy link
Copy Markdown
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets
License MIT
Doc PR Sylius/Sylius-Docs#323

Initial implementation of SyliusReviewBundle, which allows to easily manage review system. It's based on #431. "Reviewable" and "Reviewer" classes can be configured under sylius_review tag:

sylius_review:
    classes:
        product:
            subject: Sylius\Component\Core\Model\Product
            review:
                model: Sylius\Component\Product\Model\ProductReview
                form:
                    default: Sylius\Bundle\ProductBundle\Form\Type\ProductReviewType
                    admin: Sylius\Bundle\CoreBundle\Form\Type\ProductReviewAdminType
            reviewer:
                model: Sylius\Component\Core\Model\Customer

Reviews can be written by customers and guests on the product page. Newly created review has state new and can be confirmed or rejected by administrator. After confirming, it's product has average rating calculated. There are 3 possible behaviors while review creation:

  • if customer is logged in, it's automatically assigned as review author
  • if customer is not logged in, email must be provided and new customer is created based on it
  • if provided email is already used, "please, log in" message is displayed

Reviews can be also created in administration panel, with choosing product and author.

There are still some things that have to be changed to make it working perfectly:

  • change ProductReviewType to use CustomerGuestType or some similar UserBundle form type: for now there is custom transformer and listener, cause existing CustomerGuestType doesn't exactly fit ReviewBundle needs - they would probably removed after fixing this point - (EDIT: fixed in [User] Added proper listener to CustomerGuestType #3372)
  • consider how review author should be displayed on reviews list:
  • think about CustomerDeleteListener: currently every review of deleted customer is deleted as well, but I think there should have author field changed to "deleted user" or sth like this - I would appreciate any ideas about implementation
  • work on translations, cause I'm sure not everything is translated for now

Some ideas for future improvements:

  • send notification email to review author after accepting/rejecting his review
  • add possibility to send reason, why review was rejected
  • add proper info label to review if authored by user, that have already bought reviewed subject

I will be thankful for some review, cause I'm pretty sure there are some part where my code can be improved ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing license block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

None of migration files have license block ;)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All right. Anyway, after merging #3326 the dates should be set to be after 08.09.2015

@aramalipoor
Copy link
Copy Markdown
Contributor

@Zales0123 Great job, this bundle is going to be very useful since it's generic, let alone Disqus and Facebook Comments integration in future. (I wish I can find free time to help in that)

Since social media channel is very favorable to shop owners (It allows them to create a loyal customer network through that), I hope this is a good start to make Sylius both technically awesome and a powerful toolset in e-commerce marketing sense.

Congrats ;)

@Zales0123
Copy link
Copy Markdown
Contributor Author

@aramalipoor Thank you very much for you appreciation! This idea with fb comments integration is pretty cool and definitely should be considered in nearest/further future.
@pamil @stloyd @aramalipoor I'm grateful for your review, gonna fix rest of bugs or start discussion about them soon ;)

@Zales0123 Zales0123 force-pushed the sylius-review-bundle branch 2 times, most recently from bc21b3d to 43c464a Compare September 22, 2015 13:59
@michalmarcinkowski michalmarcinkowski added this to the v0.17.0 milestone Sep 22, 2015
@Zales0123 Zales0123 force-pushed the sylius-review-bundle branch 2 times, most recently from 49e9600 to 0e26b07 Compare September 23, 2015 14:28
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why we need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This variable is incremented in private addReviewRatingIfAccepted function - it's done this way to avoid passing it as reference.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The calculator shouldn't have state, so it shouldn't have any class field. This should be removed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

...-2013?

@Zales0123 Zales0123 force-pushed the sylius-review-bundle branch 2 times, most recently from 10ba6ea to 491ea1b Compare October 1, 2015 11:32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about something like:

$reviewsNumber = 0;
$sum = 0.0;

$reviews = $reviewable->getReviews()->filter(function (ReviewInterface $review) use ($sum, $reviewsNumber) {
    if (ReviewInterface::STATUS_ACCEPTED === $review->getStatus()) {
        ++$reviewsNumber;

        $sum += $review->getRating();

        return $review;
    }
});

if ($reviews->isEmpty()) {
    return 0;
}

return $sum / $reviewNumber;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can use foreach instead of filter and it will do the same job ;)

@Zales0123 Zales0123 force-pushed the sylius-review-bundle branch from 491ea1b to bdac5d6 Compare October 1, 2015 13:48
Also some minor PR review fixes
@Zales0123 Zales0123 force-pushed the sylius-review-bundle branch from bdac5d6 to 71ccbc3 Compare October 2, 2015 07:53
@pjedrzejewski pjedrzejewski removed this from the v0.17.0 milestone Dec 14, 2015
@pjedrzejewski pjedrzejewski mentioned this pull request Dec 14, 2015
@pjedrzejewski
Copy link
Copy Markdown
Contributor

Closing in favor of #3980.

@Zales0123 Zales0123 deleted the sylius-review-bundle branch October 6, 2016 20:22
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.

7 participants