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

[Admin][Customer] Seeing detailed information about customer #6596

Merged
merged 4 commits into from
Nov 4, 2016

Conversation

tuka217
Copy link
Contributor

@tuka217 tuka217 commented Oct 31, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related tickets -
License MIT

@tuka217 tuka217 changed the title [Admin][Customer] Seeing detailed information about customer [WIP][Admin][Customer] Seeing detailed information about customer Oct 31, 2016
Scenario: Seeing information about customer groups
Given the customer belongs to group "Retail"
When I view details of the customer "f.baggins@shire.me"
Then I should see the group "Retail"
Copy link
Contributor

Choose a reason for hiding this comment

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

I should see that this customer belongs to the "Retail" group?

@@ -17,20 +22,29 @@
{% endif %}
{{ 'sylius.ui.subscribed_to_newsletter'|trans }}
</div>
<div id="email-verified">
{% if customer.user.verified %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make this if inline? {{ customer.user.verified ? 'green checkmark' : 'red remove' }}

@@ -79,6 +79,7 @@ sylius:
backorder: 'Backorder'
backordered: 'Backordered'
balance_due: 'Balance due'
belonging_to_group: 'Belonging to group: '
Copy link
Contributor

@pamil pamil Oct 31, 2016

Choose a reason for hiding this comment

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

We shouldn't include : in the translation.

@tuka217 tuka217 force-pushed the customer-detail-page branch 2 times, most recently from 0e1fa7f to fb9185e Compare November 2, 2016 10:40
@tuka217 tuka217 changed the title [WIP][Admin][Customer] Seeing detailed information about customer [Admin][Customer] Seeing detailed information about customer Nov 2, 2016
{{ 'sylius.ui.subscribed_to_newsletter'|trans }}
</div>
{% if customer.user %}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about customer.user is defined?

@@ -7,30 +7,37 @@
</a>
<div class="meta">
<span class="date">{{ 'sylius.ui.customer_since'|trans }} {{ customer.createdAt|date }}</span>
<br />
{% if customer.group %}
Copy link
Contributor

Choose a reason for hiding this comment

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

customer.group is defined?

Copy link
Contributor Author

@tuka217 tuka217 Nov 3, 2016

Choose a reason for hiding this comment

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

But it should be !== null, because when variable is null then it's been already defined, WDYT?

{{ 'sylius.ui.email_verified'|trans }}
</div>
{% endif %}
{% if customer.defaultAddress %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@@ -79,6 +79,7 @@ sylius:
backorder: 'Backorder'
backordered: 'Backordered'
balance_due: 'Balance due'
belonging_to_group: 'Belonging to group'
Copy link
Contributor

Choose a reason for hiding this comment

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

Belongs?

Copy link
Contributor Author

@tuka217 tuka217 Nov 2, 2016

Choose a reason for hiding this comment

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

I meant "membership", so maybe it should be a " belongingness to groups"?

Copy link
Member

Choose a reason for hiding this comment

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

Can't it be just group_membership?

@pjedrzejewski pjedrzejewski merged commit 76797f1 into Sylius:master Nov 4, 2016
@pjedrzejewski
Copy link
Member

Thank you Ania!

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
[Admin][Customer] Seeing detailed information about customer
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

5 participants