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

#690 #498 #497 Make the check_purchase work #695

Merged
merged 16 commits into from
Jan 20, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions shopelectro/models.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import enum
import random
import string
import typing
Expand Down Expand Up @@ -135,30 +136,48 @@ class ProductFeedback(models.Model):
default='', blank=True, verbose_name=_('limitations'))


def _default_payment():
"""Default payment option is first element of first tuple in options."""
assert settings.PAYMENT_OPTIONS[0][0], 'No payment options!'
return settings.PAYMENT_OPTIONS[0][0]
class ItemsEnum(enum.EnumMeta):
"""
Provide dict-like `items` method.

https://docs.python.org/3/library/enum.html#enum-classes
"""
def items(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

so, let's implement keys() and values()?
Up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now these methods aren't needed anywhere, so it will be redundant

return [(i.name, i.value) for i in self]

def __repr__(self):
fields = ', '.join(i.name for i in self)
return f"<enum '{self.__name__}: {fields}'>"


class PaymentOptions(enum.Enum, metaclass=ItemsEnum):
cash = 'Наличные'
cashless = 'Безналичные и денежные переводы'
AC = 'Банковская карта'
PC = 'Яндекс.Деньги'
GP = 'Связной (терминал)'
AB = 'Альфа-Клик'

@staticmethod
def default():
return PaymentOptions.cash


class Order(ecommerce_models.Order):
address = models.TextField(blank=True, default='')
payment_type = models.CharField(
max_length=255,
choices=settings.PAYMENT_OPTIONS,
default=_default_payment()
choices=PaymentOptions.items(),
default=PaymentOptions.default().name,
)
comment = models.TextField(blank=True, default='')
# total price - total purchase price
revenue = models.FloatField(default=0, null=True, verbose_name=_('revenue'))

@property
def payment_type_name(self):
"""Return name for an order's payment option."""
return next(
name for option, name in settings.PAYMENT_OPTIONS
if self.payment_type == option
)
def payment_type_label(self):
"""Return label for an order's payment option."""
return PaymentOptions[self.payment_type].value

def set_positions(self, cart):
"""
Expand Down
24 changes: 24 additions & 0 deletions shopelectro/selenium/conditions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from selenium.common.exceptions import StaleElementReferenceException


def alphanumeric(text: str) -> str:
return ''.join(filter(lambda c: c.isalnum(), text))


class AlnumPresentedInValue:
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not good name because of not good semantic. is_presented_... is action, method should implement it.
But element driver.find_element(*self.locator) is object, it should have it's own class.

So, i suggest this design:

class Element:
    def __init__(self, driver: SiteDriver, locator: typing.Tuple[str, str]):
        self.driver = driver
        self.locator = locator
      
class SafeInput(Element):
   """Handles only alphanumeric symbols."""
    def contains(text: str) -> bool:
        value = driver.find_element(*self.locator).get_attribute('value')
        return alphanumeric(text) in alphanumeric(value)

class Button(Element):
    def click(self):
        self.driver.wait.until(EC.element_to_be_clickable(
            self.locator
        )).click()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@duker33 ,
AlnumPresentedInValue is an expected condition as conditions from selenium. And it may instantiate an object.

WebDriverWait can't use provided SafeInput, because it isn't a condition

AlnumInValueCondition sounds better?

Copy link
Contributor

Choose a reason for hiding this comment

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

@artemiy312 , imho selenium conditions have not the best arch in the world =)
Condition interface implementing may contain some problems, but we can resolve them.
Let's leave you code and class name (choose one). I'll try propose some changes in separated PR

"""
Check if the given text is present in the element's locator, text.

Remove all non-alphanumeric characters before checking.
"""

def __init__(self, locator, text):
self.locator = locator
self.text = text

def __call__(self, driver):
try:
text = driver.find_element(*self.locator).get_attribute('value')
return alphanumeric(self.text) in alphanumeric(text)
except StaleElementReferenceException:
return False
2 changes: 1 addition & 1 deletion shopelectro/selenium/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ class SiteDriver(Remote):
"""Provide convenient access to the site."""

def __init__(self, *, site_url, **kwargs):
super().__init__(**kwargs)
kwargs.setdefault('command_executor', settings.SELENIUM_URL)
kwargs.setdefault('desired_capabilities', DesiredCapabilities.CHROME)
super().__init__(**kwargs)

self.site_url = site_url
self.wait = WebDriverWait(self, settings.SELENIUM_WAIT_SECONDS)
Expand Down
2 changes: 2 additions & 0 deletions shopelectro/selenium/elements/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
from .input import Input
from .button import Button
from .product_card import ProductCard
15 changes: 15 additions & 0 deletions shopelectro/selenium/elements/button.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
from shopelectro.selenium.driver import SiteDriver

from selenium.webdriver.support import expected_conditions as EC


class Button:

def __init__(self, driver: SiteDriver, locator):
self.driver = driver
self.locator = locator

def click(self):
self.driver.wait.until(EC.element_to_be_clickable(
self.locator
)).click()
22 changes: 22 additions & 0 deletions shopelectro/selenium/elements/input.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import re

from shopelectro.selenium.conditions import AlnumPresentedInValue
from shopelectro.selenium.driver import SiteDriver

from selenium.webdriver.support import expected_conditions as EC


class Input:

def __init__(self, driver: SiteDriver, locator):
self.driver = driver
self.locator = locator

def send_keys(self, keys: str):
keys = str(keys)
if not re.match(r'[\w\d\s\-_]+', keys, re.I | re.U):
raise ValueError('Remove special symbols from text')
el = self.driver.wait.until(EC.element_to_be_clickable(self.locator))
el.clear()
el.send_keys(keys)
self.driver.wait.until(AlnumPresentedInValue(self.locator, keys))
13 changes: 7 additions & 6 deletions shopelectro/selenium/elements/product_card.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from selenium.driver import SiteDriver
from shopelectro.selenium.elements import Button
from shopelectro.selenium.driver import SiteDriver

from selenium.webdriver.common.by import By
from selenium.webdriver.support import expected_conditions as EC


class ProductCard:
Expand All @@ -14,9 +14,10 @@ def __init__(self, driver: SiteDriver, card_index: int):
:param int card_index: The index number of the product card at a category page
"""
self.driver = driver
self.button_xpath = f'//*[@id="products-wrapper"]/div[{card_index}]/div[2]/div[5]/button'
self.button = Button(
self.driver,
(By.XPATH, f'//*[@id="products-wrapper"]/div[{card_index}]/div[2]/div[5]/button')
)

def add_to_cart(self):
self.driver.wait.until(
EC.visibility_of_element_located((By.XPATH, self.button_xpath))
).click()
self.button.click()
3 changes: 2 additions & 1 deletion shopelectro/selenium/pages/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from .page import Page

from .category import CategoryPage
from .order import OrderPage
from .page import Page
3 changes: 2 additions & 1 deletion shopelectro/selenium/pages/category.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ def product_cards(self) -> typing.List[ProductCard]:
raise NotImplementedError

def add_to_cart(self, product_cards: typing.List[ProductCard]=None):
product_cards = product_cards or self.product_cards()[:6]
default_cards = [ProductCard(self.driver, i) for i in range(1, 7)]
product_cards = product_cards or default_cards
for card in product_cards:
card.add_to_cart()
37 changes: 31 additions & 6 deletions shopelectro/selenium/pages/order.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
from shopelectro.models import PaymentOptions
from shopelectro.selenium.elements import Input, Button
from shopelectro.selenium.pages import Page

from selenium.webdriver.common.by import By

from pages.models import CustomPage

# @todo #682:120m Implement and reuse shopelectro.selenium.OrderPage for selenium tests.
Expand All @@ -9,16 +13,37 @@ class OrderPage(Page):

def __init__(self, driver):
super().__init__(driver)
self.submit_button = Button(self.driver, (By.ID, 'submit-order'))

@property
def path(self):
return CustomPage.objects.get(slug='order').url

def fill_contacts(self, contacts):
raise NotImplementedError
def fill_contacts(
self, name='Name', city='Санкт-Петербург', phone='2222222222', email='test@test.test',
):
contacts = {
'id_name': name,
'id_city': city,
'id_phone': phone,
'id_email': email,
}

def make_order(self):
raise NotImplementedError
for id_, value in contacts.items():
Input(self.driver, (By.ID, id_)).send_keys(value)

def select_payment_type(self):
raise NotImplementedError
def make_order(self):
Copy link
Contributor

@duker33 duker33 Jan 19, 2019

Choose a reason for hiding this comment

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

Suggested change
def make_order(self):
def order(self):

Maybe it's taste, up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems something went wrong and i can see the suggested change

Copy link
Contributor

Choose a reason for hiding this comment

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

@artemiy312 , sorry, fixed my comment above

Copy link
Contributor Author

@ArtemijRodionov ArtemijRodionov Jan 20, 2019

Choose a reason for hiding this comment

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

order may have too many means: some representation of an order, order of positions, order as an action. In other words, it may looks both like a command and a query, so i'd like to leave it as the explicit command

self.submit_button.click()

def select_payment_type(self, payment_option: PaymentOptions):
if payment_option not in PaymentOptions:
raise ValueError(
'An invalid payment type provided.'
f'It should be one of: {PaymentOptions}'
)

item = Button(
self.driver,
(By.CSS, f'input[name="payment_type"][value="{payment_type.name}"]'),
)
item.click()
5 changes: 4 additions & 1 deletion shopelectro/selenium/pages/page.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from shopelectro.selenium import SiteDriver

from selenium.webdriver.common.by import By
from selenium.webdriver.support import expected_conditions as EC


class Page:
"""
Expand All @@ -9,7 +12,7 @@ class Page:
"""

def __init__(self, driver: SiteDriver):
if not isinstance(self.driver, SiteDriver):
if not isinstance(driver, SiteDriver):
raise TypeError('Driver must be an instance of shopelectro.selenium.SiteDriver')
self.driver = driver
self.path: str
Expand Down
12 changes: 1 addition & 11 deletions shopelectro/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
For the full list of settings and their values, see
https://docs.djangoproject.com/en/1.9/ref/settings/
"""

import enum
import os
import socket
from datetime import datetime
Expand Down Expand Up @@ -244,16 +244,6 @@
# Used to retrieve instances in ecommerce.Cart
CART_ID = 'cart'

# Used to define choices attr in definition of Order.payment_type field
PAYMENT_OPTIONS = (
('cash', 'Наличные'),
('cashless', 'Безналичные и денежные переводы'),
('AC', 'Банковская карта'),
('PC', 'Яндекс.Деньги'),
('GP', 'Связной (терминал)'),
('AB', 'Альфа-Клик'),
)

# It is fake-pass. Correct pass will be created on `docker-compose up` stage from `docker/.env`
YANDEX_SHOP_PASS = os.environ.get('YANDEX_SHOP_PASS', 'so_secret_pass')

Expand Down
5 changes: 2 additions & 3 deletions shopelectro/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ def update_catalog():
collect_static()
]

# @todo #682:60m Implement check_purchase.
# Handle errors. Report failed attempts.
# Schedule it in the celery beat.
# @todo #690:60m Handle errors in check_purchase.
# Report failed attempts. Schedule it in the celery beat.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Report failed attempts. Schedule it in the celery beat.
# Report failed attempts with asyncio.

Maybe we'll write new functionality throw asyncio? This will allow us to move all functionality to asyncio bit by bit.
Up to you

Copy link
Contributor

Choose a reason for hiding this comment

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

this code is too low related to check_purchase, i suppose.
But it's strongly related to "Implement OrderPage" and some sort of "Implement CategoryPage".
So, let's close and reopen these tasks instead of check_purchase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, let's close and reopen these tasks instead of check_purchase?

I believe, this PR is related to check_purchase implementation, because check_purchase wouldn't work without this PR. Each change from this PR has brought check_purchase to its final state.

May be Implement check_purchase. isn't the most accurate description of the issue, but if it is important, then i can edit the original description to make the check_purchase work. Is it sound ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

@artemiy312 , seems interesting case here =)
You resolved issue "check purchase", and i agree with that.
But to solve it you created code of selenium.OrderPage, selenium.CategoryPage classes and it's elements.

So, we should rename your PR to some sort of Implement base selenium page elements and leave your @todo cases fixes. PR name is not so necessary, it's up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just have renamed names of issue and PR. Now it looks fine



@app.task
Expand Down
2 changes: 0 additions & 2 deletions shopelectro/tests/tests_selenium.py
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,6 @@ def buy_products(self):
).click()

def select_payment_type(self, payment_type='cash'):
# @todo #473:15m Move `payment_type` to dict or dataclass
input_item = self.browser.find_element_by_css_selector(
f'input[name="payment_type"][value="{payment_type}"]'
)
Expand All @@ -685,7 +684,6 @@ def expected_conditions(browser):
insert_value('id_email', 'test@test.test')

def submit_form(self):
# @todo #473:30m Hide all form processing methods to a separated class.
self.click((By.ID, 'submit-order'))

def test_table_is_presented_if_there_is_some_products(self):
Expand Down
2 changes: 1 addition & 1 deletion templates/ecommerce/order/email.html
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@

<ul style="padding:0;list-style:none">
<li style="margin:0 0 1em">Тип платежа:
<strong style="font-weight:bold">{{ order.payment_type_name }}</strong>
<strong style="font-weight:bold">{{ order.payment_type_label }}</strong>
</li>
<li style="margin:0 0 1em">Телефон:
<strong style="font-weight:bold">{{ order.phone }}</strong>
Expand Down
2 changes: 1 addition & 1 deletion templates/ecommerce/yandex_order_email.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
</tr>
<tr>
<td>Способ оплаты</td>
<td>{{ order.payment_type_name }}</td>
<td>{{ order.payment_type_label }}</td>
</tr>
{% if order.comment %}
<tr>
Expand Down