feat: Addition and Updation of billing related fields #6447

kushthedude · 2019-09-15T03:07:56Z

Fixes #6444

app/api/orders.py

kushthedude · 2019-09-15T03:09:34Z

@iamareebjamal Please Review

app/api/orders.py

kushthedude · 2019-09-16T04:09:35Z

@iamareebjamal I can't debug the issue with dredd, Can you please point out what's getting wrong, As on local the server is running well

iamareebjamal · 2019-09-16T06:46:42Z

Traceback (most recent call last):
  File "manage.py", line 97, in <module>
    manager.run()
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/flask_script/__init__.py", line 417, in run
    result = self.handle(argv[0], argv[1:])
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/flask_script/__init__.py", line 386, in handle
    res = handle(*args, **config)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/flask_script/commands.py", line 216, in __call__
    return self.run(*args, **kwargs)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/flask_migrate/__init__.py", line 95, in wrapped
    f(*args, **kwargs)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/flask_migrate/__init__.py", line 359, in heads
    resolve_dependencies=resolve_dependencies)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/command.py", line 429, in heads
    heads = script.get_revisions(script.get_heads())
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/script/base.py", line 316, in get_heads
    return list(self.revision_map.heads)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/util/langhelpers.py", line 230, in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/script/revision.py", line 72, in heads
    self._revision_map
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/util/langhelpers.py", line 230, in __get__
    obj.__dict__[self.__name__] = result = self.fget(obj)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/script/revision.py", line 123, in _revision_map
    for revision in self._generator():
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/script/base.py", line 109, in _load_revisions
    script = Script._from_filename(self, vers, file_)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/script/base.py", line 887, in _from_filename
    module = util.load_python_file(dir_, filename)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/util/pyfiles.py", line 98, in load_python_file
    module = load_module_py(module_id, path)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/alembic/util/compat.py", line 174, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 674, in exec_module
  File "<frozen importlib._bootstrap_external>", line 781, in get_code
  File "<frozen importlib._bootstrap_external>", line 741, in source_to_code
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/travis/build/fossasia/open-event-server/migrations/versions/rev-2019-09-15-08:29:32-d1c2b8711223_.py", line 27
    nullable=False)

migrations/versions/rev-2019-09-15-08:29:32-d1c2b8711223_.py

iamareebjamal · 2019-09-16T06:49:20Z

The build didn't stop due to this error - #5988

codecov · 2019-09-16T07:28:03Z

Codecov Report

Merging #6447 into development will increase coverage by <.01%.
The diff coverage is 66.66%.

@@               Coverage Diff               @@
##           development    #6447      +/-   ##
===============================================
+ Coverage        65.02%   65.03%   +<.01%     
===============================================
  Files              296      296              
  Lines            15250    15248       -2     
===============================================
  Hits              9917     9917              
+ Misses            5333     5331       -2

Impacted Files	Coverage Δ
app/api/orders.py	`26.81% <0%> (+0.13%)`	⬆️
app/models/order.py	`90% <100%> (ø)`	⬆️
app/models/event.py	`78.92% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bde88a4...f07085a. Read the comment docs.

app/api/orders.py

houndci-bot · 2019-09-24T16:22:17Z

app/api/orders.py

+	if data.get('is_billing_enabled') and not (data.get('company') and data.get('address') 
+	                and data.get('city') and data.get('zipcode') and data.get('country')):
+		raise UnprocessableEntity({'pointer': '/data/attributes/is_billing_enabled'},
+                                  "Billing information is incomplete.")


continuation line over-indented for visual indent
indentation contains mixed spaces and tabs

app/api/orders.py

houndci-bot · 2019-09-24T16:22:18Z

app/api/orders.py

+
+def check_billing_info(data, order):
+	if data.get('is_billing_enabled') and not (data.get('company') and data.get('address') 
+	                and data.get('city') and data.get('zipcode') and data.get('country')):


continuation line under-indented for visual indent
indentation contains mixed spaces and tabs
indentation contains tabs

app/api/orders.py

kushthedude · 2019-09-24T17:22:59Z

@iamareebjamal Please Review

iamareebjamal · 2019-11-24T00:04:01Z

Is it ready for merging now?

kushthedude · 2019-11-24T00:05:51Z

Is it ready for merging now?

Yes it is working now

iamareebjamal · 2019-11-24T00:10:34Z

app/api/orders.py

        :return:
        """
-        if data.get('amount') or data.get('is_billing_enabled'):
+        if data.get('amount') or data.get('is_billing_enabled') or order.event.is_billing_info_mandatory:


There's a problem, this will require billing info on any paid order, whereas it is explicitly stated in the issue to make it optional unless is_billing_info_mandatory is True.

Skipping this PR for now, will add in minor version

Merging it after release would be better I guess too.

houndci-bot · 2019-11-24T02:00:12Z

app/api/orders.py

        :return:
        """
-        if data.get('amount') or data.get('is_billing_enabled'):
+        if data.get('amount') and (data.get('is_billing_enabled') or order.event.is_billing_info_mandatory) :


whitespace before ':'

Please don't mark hound violations as resolved

kushthedude · 2019-11-25T06:32:41Z

@iamareebjamal Updated the PR and tested with making orders and requests too. Also Frontend PR is done too fossasia/open-event-frontend#3654

iamareebjamal · 2019-11-25T06:38:33Z

app/api/orders.py

        :return:
        """
-        if data.get('amount') or data.get('is_billing_enabled'):
+        if data.get('amount') and (data.get('is_billing_enabled') or order.event.is_billing_info_mandatory) :


This will again, require billing info even if the organizer has asked not to. Please read the issue again. I don't think you understand what the goal is.

Can you tell me what is the goal of this PR?

This PR goal is, If a order is paid and organiser has made billing mandatory , user has to fill it.
And if its not mandatory then user has option to fill or not

Also Billing information is only needed for paid order hence a condition for data.get(amount)

what

If organiser has not asked for billing info, Then it wont require as I have used and over there

@iamareebjamal

Yeah, and this is going to require billing info even when organizer has set event.is_billing_info_mandatory = False

@iamareebjamal

a) Only make billing info required if the organizer chose "Require billing info.." (in Wizard). If billing > info is required always show the fields. If the organizer did not require it, show it as an option > > > using a tick box. -> So, if the organizer does not require it, the user can still add the

Yeah, and this is going to require billing info even when organizer has set event.is_billing_info_mandatory = False

It will billing info only then when the user wants to enter it

iamareebjamal

Understand the issue first

kushthedude · 2019-11-25T06:41:40Z

Understand the issue first

Please see the comments. Its working as expected

iamareebjamal · 2019-11-25T06:45:04Z

Please see the comments. Its working as expected

data.get('amount') and (data.get('is_billing_enabled') or order.event.is_billing_info_mandatory)

What's the use of data.get('is_billing_enabled') here?

kushthedude · 2019-11-25T06:47:58Z

Please see the comments. Its working as expected

data.get('amount') and (data.get('is_billing_enabled') or order.event.is_billing_info_mandatory)

What's the use of data.get('is_billing_enabled') here?

If billing is not mandatory, then its dependent upon the user to enter or not.

kushthedude · 2019-11-25T06:48:51Z

@iamareebjamal

a) Only make billing info required if the organizer chose "Require billing info.." (in Wizard). If billing > info is required always show the fields. If the organizer did not require it, show it as an option > > > using a tick box. -> So, if the organizer does not require it, the user can still add the

iamareebjamal · 2019-11-25T06:50:02Z

It's not necessary that the user will set is_billing_enabled as True or not. There is something else which needs to be thought in order to ensure checking of billing info before storing it.

kushthedude · 2019-11-25T06:51:09Z

It's not necessary that the user will set is_billing_enabled as True or not. There is something else which needs to be thought in order to ensure checking of billing info before storing it.

If user sets it as true when Billing_info is not mandatory, Then he can enter the data. If he doesn't he will not enter ?

iamareebjamal · 2019-11-25T06:52:03Z

app/api/orders.py

-                                               data.get('zipcode') and data.get('country')):
+                                  "Billing information is mandatory for this order.")
+    if data.get('is_billing_enabled') and not (data.get('company') and data.get('address')
+	                and data.get('city') and data.get('zipcode') and data.get('country')):


Revert this please. The previous format was according to PEP8

iamareebjamal · 2019-11-25T06:52:08Z

app/api/orders.py

-                                  "Billing information is mandatory for paid orders")
-    if data.get('is_billing_enabled') and not (data.get('company') and data.get('address') and data.get('city') and
-                                               data.get('zipcode') and data.get('country')):
+                                  "Billing information is mandatory for this order.")


Remove period from the end

iamareebjamal · 2019-11-25T06:52:29Z

app/api/orders.py

+	                and data.get('city') and data.get('zipcode') and data.get('country')):
        raise UnprocessableEntity({'pointer': '/data/attributes/is_billing_enabled'},
-                                  "Billing information incomplete")
+                                  "Billing information is incomplete.")


Remove the period from the end

iamareebjamal · 2019-11-25T06:57:38Z

If user sets it as true when Billing_info is not mandatory, Then he can enter the data. If he doesn't he will not enter ?

For now, it is fine. But this may happen - a user enters billing info and does not set is_billing_enabled as true, then incomplete and inconsistent data may be stored in the DB. But, that can be handled later

kushthedude · 2019-11-25T06:58:47Z

If user sets it as true when Billing_info is not mandatory, Then he can enter the data. If he doesn't he will not enter ?

For now, it is fine. But this may happen - a user enters billing info and does not set is_billing_enabled as true, then incomplete and inconsistent data may be stored in the DB. But, that can be handled later

@iamareebjamal Made the requested changes.

niranjan94 · 2019-11-25T07:03:47Z

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- app/api/orders.py  1

See the complete overview on Codacy

houndci-bot reviewed Sep 15, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

iamareebjamal reviewed Sep 15, 2019

View reviewed changes

app/api/orders.py Show resolved Hide resolved

houndci-bot reviewed Sep 15, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

kushthedude force-pushed the fields branch from b41b9b0 to eecd9e6 Compare September 15, 2019 16:58

kushthedude requested a review from iamareebjamal September 15, 2019 16:59

shreyanshdwivedi reviewed Sep 15, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

kushthedude force-pushed the fields branch from eecd9e6 to 9bfde57 Compare September 15, 2019 18:34

houndci-bot reviewed Sep 15, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

app/api/orders.py Outdated Show resolved Hide resolved

enh: Addition of billing info related fields

fcf6de4

kushthedude force-pushed the fields branch from 62179aa to fcf6de4 Compare September 15, 2019 18:36

kushthedude added the enhancement label Sep 15, 2019

iamareebjamal reviewed Sep 16, 2019

View reviewed changes

migrations/versions/rev-2019-09-15-08:29:32-d1c2b8711223_.py Show resolved Hide resolved

Update rev-2019-09-15-08:29:32-d1c2b8711223_.py

f0641e0

Update rev-2019-09-15-08:29:32-d1c2b8711223_.py

936264a

iamareebjamal reviewed Sep 16, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

iamareebjamal reviewed Sep 16, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

kushthedude added 2 commits September 24, 2019 11:01

Merge branch 'development' into fields

68ed374

Adding another function

2f50044

houndci-bot reviewed Sep 24, 2019

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

app/api/orders.py Outdated Show resolved Hide resolved

kushthedude requested a review from iamareebjamal September 24, 2019 16:11

Update orders.py

249d067

houndci-bot reviewed Sep 24, 2019

View reviewed changes

kushthedude added 2 commits September 25, 2019 19:22

Merge branch 'development' into fields

2b65543

Update orders.py

ae86e49

iamareebjamal reviewed Nov 24, 2019

View reviewed changes

Final fixes

cb7265d

houndci-bot reviewed Nov 24, 2019

View reviewed changes

Merge branch 'development' into fields

873980c

kushthedude mentioned this pull request Nov 24, 2019

feat: Billing Information Enhancements and Updations fossasia/open-event-frontend#3654

Merged

iamareebjamal reviewed Nov 25, 2019

View reviewed changes

Hound violation

c74d885

iamareebjamal requested changes Nov 25, 2019

View reviewed changes

iamareebjamal reviewed Nov 25, 2019

View reviewed changes

Syntax enhancement

f07085a

iamareebjamal approved these changes Nov 25, 2019

View reviewed changes

iamareebjamal changed the title ~~enh: Addition and Updation of billing related fields~~ feat: Addition and Updation of billing related fields Nov 25, 2019

auto-label bot added the feature label Nov 25, 2019

iamareebjamal merged commit 4f32edf into fossasia:development Nov 25, 2019

kushthedude deleted the fields branch November 25, 2019 07:04

codedsun pushed a commit to codedsun/open-event-server that referenced this pull request Dec 22, 2019

feat: Addition and Updation of billing related fields (fossasia#6447)

2ddaae7

feat: Addition and Updation of billing related fields #6447

feat: Addition and Updation of billing related fields #6447

Uh oh!

Conversation

kushthedude commented Sep 15, 2019

Uh oh!

Uh oh!

kushthedude commented Sep 15, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kushthedude commented Sep 16, 2019

Uh oh!

iamareebjamal commented Sep 16, 2019

Uh oh!

Uh oh!

iamareebjamal commented Sep 16, 2019

Uh oh!

codecov bot commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kushthedude commented Sep 24, 2019

Uh oh!

iamareebjamal commented Nov 24, 2019

Uh oh!

kushthedude commented Nov 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kushthedude commented Nov 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iamareebjamal left a comment

Choose a reason for hiding this comment

Uh oh!

kushthedude commented Nov 25, 2019

Uh oh!

iamareebjamal commented Nov 25, 2019

Uh oh!

kushthedude commented Nov 25, 2019

Uh oh!

kushthedude commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

codecov bot commented Sep 16, 2019 •

edited

Loading

kushthedude commented Nov 25, 2019 •

edited

Loading