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

ArtemijRodionov · 2019-01-17T18:24:56Z

duker33

one arch question, few moments about subtasks.

In a whole new design looks perfect!

duker33 · 2019-01-19T06:17:32Z

shopelectro/selenium/conditions.py

+    return ''.join(filter(lambda c: c.isalnum(), text))
+
+
+class AlnumPresentedInValue:


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()

@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?

@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

duker33 · 2019-01-19T06:18:43Z

shopelectro/selenium/pages/order.py


-    def select_payment_type(self):
-        raise NotImplementedError
+    def make_order(self):


Suggested change

def make_order(self):

def order(self):

Maybe it's taste, up to you

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

@artemiy312 , sorry, fixed my comment above

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

duker33 · 2019-01-19T06:19:38Z

shopelectro/models.py

-    return settings.PAYMENT_OPTIONS[0][0]
+class PairIterEnum(enum.EnumMeta):
+
+    def __iter__(self):


leave plz comment about this logic.
Metaclasses is not widely used feature, so it's quite hard to understand what happens there

duker33 · 2019-01-19T06:20:06Z

shopelectro/models.py

+
+    def __repr__(self):
+        keys = ', '.join(next(zip(*PaymentOptions)))
+        return f"<enum '{self.__name__}: {names}'>"


you define keys, but here use undefined names. Is it right?

No, it is typo

duker33 · 2019-01-19T06:21:28Z

shopelectro/tasks.py

-#  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.


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

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?

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?

@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

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

duker33 · 2019-01-20T06:46:17Z

shopelectro/models.py

-            yield i.name, i.value
+    https://docs.python.org/3/library/enum.html#enum-classes
+    """
+    def items(self):


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

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

ArtemijRodionov added 3 commits January 17, 2019 20:43

Fix current implementation of selenium classes

7fe193f

Create Input element class for selenium

e58655e

Implement fill_cotacts and make_order methods of OrderPage class

9b94132

ArtemijRodionov self-assigned this Jan 17, 2019

ArtemijRodionov requested a review from duker33 January 17, 2019 18:24

ArtemijRodionov changed the title ~~#690 check purchase~~ #690 #498 check purchase Jan 17, 2019

ArtemijRodionov added 5 commits January 17, 2019 21:28

Self-review fixes

a333d77

Create PaymentOptions enum

2ec22a0

Create Button class, implement OrderPage.select_payment_type

351387a

Remove todo about payment_types

cb301ee

Apply linter rules

0010a5d

ArtemijRodionov changed the title ~~#690 #498 check purchase~~ #690 #498 #497 check purchase Jan 18, 2019

ArtemijRodionov added 3 commits January 18, 2019 17:53

Self-review fixes

6057aca

Move PaymentOptions to shopelectro.models

24ae611

Fix a typo

d4c59ed

ArtemijRodionov force-pushed the 690-check-purchase branch from bb3e0c5 to d4c59ed Compare January 18, 2019 20:27

Apply linter rules

d0daa04

duker33 requested changes Jan 19, 2019

View reviewed changes

ArtemijRodionov added 4 commits January 19, 2019 21:42

Provide the comment for EnumMeta class.

46d2773

Remove excess import from base.py

de9a7ad

Fix typo

d44644a

Apply linter rules

4e7fce1

duker33 reviewed Jan 20, 2019

View reviewed changes

duker33 approved these changes Jan 20, 2019

View reviewed changes

ArtemijRodionov changed the title ~~#690 #498 #497 check purchase~~ #690 #498 #497 Make the check_purchase work Jan 20, 2019

ArtemijRodionov merged commit ce492b1 into master Jan 20, 2019

ArtemijRodionov deleted the 690-check-purchase branch January 20, 2019 13:54

duker33 mentioned this pull request Jan 26, 2019

#704 check purchase #709

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ArtemijRodionov commented Jan 17, 2019 •

edited

Loading

duker33 left a comment

duker33 Jan 19, 2019

ArtemijRodionov Jan 19, 2019

duker33 Jan 20, 2019

duker33 Jan 19, 2019 •

edited

Loading

ArtemijRodionov Jan 19, 2019

duker33 Jan 20, 2019

ArtemijRodionov Jan 20, 2019 •

edited

Loading

duker33 Jan 19, 2019

duker33 Jan 19, 2019

ArtemijRodionov Jan 19, 2019

duker33 Jan 19, 2019

duker33 Jan 19, 2019

ArtemijRodionov Jan 19, 2019

duker33 Jan 20, 2019

ArtemijRodionov Jan 20, 2019

duker33 Jan 20, 2019

ArtemijRodionov Jan 20, 2019

		return ''.join(filter(lambda c: c.isalnum(), text))


		class AlnumPresentedInValue:

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

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

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

Conversation

ArtemijRodionov commented Jan 17, 2019 • edited Loading

duker33 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

duker33 Jan 19, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtemijRodionov Jan 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtemijRodionov commented Jan 17, 2019 •

edited

Loading

duker33 Jan 19, 2019 •

edited

Loading

ArtemijRodionov Jan 20, 2019 •

edited

Loading