Skip to content

Conversation

@vladkhard
Copy link

@vladkhard vladkhard commented Jan 16, 2018

This change is Reviewable

@VDigitall
Copy link
Member

Зміни мають іти з тестами


def get_resource_item_historical(self, id, headers=None):
return self._get_resource_item('{}/{}/historical'.format(self.prefix_path, id), headers=headers)
def get_resource_item_historical(self, id, revision, headers=None):
Copy link
Member

Choose a reason for hiding this comment

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

Я б не робив revision обовязковим

Copy link
Author

@vladkhard vladkhard Jan 16, 2018

Choose a reason for hiding this comment

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

Чому? Це викoристовується тільки для хісторікала, реквест без ревізії не має сенсу - щоб взяти актуальну версію документа, досить використати get_resource_item

Copy link
Member

@VDigitall VDigitall Jan 16, 2018

Choose a reason for hiding this comment

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

Щоб в еджі замінити ось це

        response = client.request(
            'GET', '{}/{}/historical'.format(api_client_dict['client'].prefix_path, id)
        )
        revisions_number = int(response.headers['x-revision-n'])

@VDigitall
Copy link
Member

@vladkhard потрібно ще тести.

def side_effect(_, headers):
return item if headers["x-revision-n"] else response

self.client._get_resource_item = MagicMock(side_effect=side_effect)
Copy link
Member

Choose a reason for hiding this comment

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

self.client.get_resource_item_historical(item_id, revision=0)
self.client.get_resource_item_historical(item_id, revision=revisions_limit + 1)
self.client.get_resource_item_historical(item_id, revision=None)

Copy link
Member

@VDigitall VDigitall Feb 9, 2018

Choose a reason for hiding this comment

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

Тут краще загорнути у for і переконатись що ми дійсно отримуємо 404 всі рази.

import unittest

from openprocurement_client.tests import tests, tests_sync
from openprocurement_client.tests import tests, tests_sync, tests_api_base_client
Copy link
Member

Choose a reason for hiding this comment

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

А де сам тест?

Copy link
Author

Choose a reason for hiding this comment

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

упс
вже додав

@VDigitall
Copy link
Member

@vladkhard тест провалився

@coveralls
Copy link

coveralls commented Feb 14, 2018

Coverage Status

Coverage increased (+0.2%) to 85.581% when pulling df58ed2 on vladkhard:use_requests into 867fc7d on openprocurement:use_requests.

item_id, revision=revisions_limit - 1), actual_response)

for revision in (0, revisions_limit + 1, None):
try:
Copy link
Member

Choose a reason for hiding this comment

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

for revision in (0, revisions_limit + 1, None):
    with self.assertRaises(InvalidResponse) as e:
        self.client.get_resource_item_historical(item_id, revision=revision)
    self.assertEqual(e.exception.status_code, 404)

@VDigitall
Copy link
Member

@VDigitall
Copy link
Member

@vladkhard, дані зміни вже не акутальні оскільки historical вміє працювати з датою, а якщо передати обидва заголовки то буде помилка.

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