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

Update Insider Auction models and views for auction substatuses #45

Open
wants to merge 7 commits into
base: production
Choose a base branch
from

Conversation

Scandie
Copy link
Contributor

@Scandie Scandie commented Jan 12, 2018

No description provided.

from schematics.types.serializable import serializable
from zope.interface import implementer
from openprocurement.api.models import (
Model, ListType
)

from openprocurement.api.utils import calculate_business_date
from openprocurement.api.models import get_now, Value, Period, TZ, SANDBOX_MODE
from openprocurement.api.models import get_now, Value, Period, TZ, SANDBOX_MODE, schematics_embedded_role
Copy link
Member

Choose a reason for hiding this comment

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

schematics_embedded_role імпортовано, але не використовується

@@ -3,17 +3,17 @@
from schematics.types import StringType
from schematics.types.compound import ModelType
from schematics.exceptions import ValidationError
from schematics.transforms import whitelist
from schematics.transforms import blacklist, whitelist
Copy link
Member

Choose a reason for hiding this comment

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

blacklist імпортовано, але не використовується

auction.bids = []
auction.status = 'cancelled'

def cancel_lot(self, cancellation=None):
Copy link
Member

Choose a reason for hiding this comment

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

Що змінилося у cancel_lot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if auction.status == 'active.auction' and all ... > if 'active.auction' in auction.status and all ...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Пропоную то все винести в окрему функцію, або у валідатор і змінити самі views з використанням валідатору, щоб не копіювати кучу коду, якщо таке можливо звичайно.

@coveralls
Copy link

coveralls commented Jan 18, 2018

Coverage Status

Coverage increased (+2.8%) to 93.817% when pulling 48440fc on Scandie:a448140157209084_active.auction_substatuses into 3e1c03d on openprocurement:production.

@@ -4,7 +4,9 @@
DUTCH_PERIOD = timedelta(minutes=405)
QUICK_DUTCH_PERIOD = timedelta(minutes=10)

TENDER_PERIOD_STATUSES = ['active.tendering', 'active.auction']
TENDER_PERIOD_STATUSES = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Я б ці підстатуси розідлив. Бо active.tendering наприклад - це статус процедури, а active.auction.dutch то є підстатус аукціону. А константа за це відповідає одна TENDER_PREIOD_STATUSES не дуже логічно.

"""
if self.request.authenticated_role == 'bid_owner':
return {'data': self.request.context.serialize('view')}
if self.request.validated['auction_status'] in TENDER_PERIOD_STATUSES:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Винеси у валідатор.

from openprocurement.auctions.dgf.views.financial.tender_document import (
FinancialAuctionDocumentResource,
)
from openprocurement.auctions.insider.constants import TENDER_PERIOD_STATUSES
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused import.

auction.bids = []
auction.status = 'cancelled'

def cancel_lot(self, cancellation=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Пропоную то все винести в окрему функцію, або у валідатор і змінити самі views з використанням валідатору, щоб не копіювати кучу коду, якщо таке можливо звичайно.


"""
auction = self.request.validated['auction']
if self.request.validated['auction_status'] in TENDER_PERIOD_STATUSES:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Знову ж таки валідатор.

@Scandie
Copy link
Contributor Author

Scandie commented Feb 22, 2018

@@ -27,7 +30,7 @@ class InsiderAuctionAuctionResource(FinancialAuctionAuctionResource):

@json_view(permission='auction')
def collection_get(self):
if self.request.validated['auction_status'] not in ['active.tendering', 'active.auction']:
if self.request.validated['auction_status'] not in TENDER_PERIOD_STATUSES:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Хіба тобі потрібна константа TENDER_PERIOD_STATUSES, коли в тебе є https://github.com/openprocurement/openprocurement.auctions.core/pull/27/files#diff-5553f0cc7a26748bc279b460b148bef5R6

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yarsanich, не знав як логічно виділити групу статусів, за яких в кожній з процедур можна робити GET запити до аукціону.

Scandie pushed a commit to Scandie/openprocurement.auctions.insider that referenced this pull request May 9, 2018
…29667_add_recomposed_status

Add recomposed status and expand tests
Scandie pushed a commit to Scandie/openprocurement.auctions.insider that referenced this pull request Jul 18, 2018
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.

4 participants