Create customer pos #6
Conversation
This reverts commit b6fd7b1
Move qb type to realm mixin
bedilbek
left a comment
bedilbek
left a comment
There was a problem hiding this comment.
I am waiting for your response to the comments I have left it here
| def get_request(self): | ||
| obj_class = import_object_cls(self.qb_resource) | ||
| service = obj_class.get_service()() | ||
| service.qb_type = self.realm.qb_type # to generate proper XML |
There was a problem hiding this comment.
I am curious, whether we really have to store qb_type as a property of Service?
There was a problem hiding this comment.
qb_type is used int _prepare_request() in services/base.py, to generate XML
I just needed a way to pass qb_type to Service.
This is the best I could came up with. I don't mind if you have a better solution to pass qb_type.
This :
@property
def qb_type(self):
raise NotImplementedError("qb_type is a required")
makes sure it is implemented in QBDTaskMixin
| obj_class = None | ||
|
|
||
| def __init__(self, response, hresult, message): | ||
| def __init__(self, response, hresult, message, realm): |
There was a problem hiding this comment.
You have to rebase your branch from master because master branch is diverted, and I came to conclusion that I do not have to store realm attribute inside ResponseProcessor, but instead, I have to pass it as an argument while processing ResponseProcessor. What do you think about it?
There was a problem hiding this comment.
I agree, I found that you _process upon initialization of the object, so I had to pass realm in __init__.
can you show me code on how to achieve that?
BTW I believe this branch is rebased but from master, however I am still learning git and github.
|
|
||
| def _process(self): | ||
| def _process(self, qb_type): | ||
| xml_type = utils.get_xml_type(qb_type) |
There was a problem hiding this comment.
I actually understand this situation, because after adding POS integration, we will not have static xml type any more. So, maybe refactoring of Processors also needed. Hmm, interesting. The more project gets complex, the more I contemplate that I have not enough time to contribute and enough skills to envision the future path of the project.
| module_path, class_name = val.rsplit('.', 1) | ||
| module = import_module(module_path) | ||
| return getattr(module, class_name) | ||
| if val: |
There was a problem hiding this comment.
why we need check val for existence?
There was a problem hiding this comment.
I couldn't understand the reason, but while processing the response, it was throwing an error, because val was empty was empty sometimes
|
|
||
| from django_quickbooks.views import QuickBooksService, Support | ||
|
|
||
| app_name = "django_quickbooks" |
There was a problem hiding this comment.
I believe we do not need to add app_name.
There was a problem hiding this comment.
we don't need it, but it is nice to have it, to avoid confusion when referring to any url.
i.e. when accessing the url from the user project, in case they have similar url confs
| if hresult: | ||
| print("hresult=" + hresult) | ||
| print("message=" + message) | ||
| # FIXME: I know this is a hacky way to get around encoding, needs a better solution |
There was a problem hiding this comment.
Does the response come explicitly in windows-1252 encoding?
There was a problem hiding this comment.
yes, please take a look at data/pos/item_inventory_add_response.xml
2 participants
Let me know if you have any comments