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

[Enhance] Subscription module #10089

Merged
merged 10 commits into from
Sep 7, 2017

Conversation

rohitwaghchaure
Copy link
Collaborator

@rohitwaghchaure rohitwaghchaure commented Jul 25, 2017

  • re-label "Base doctype" to "Reference Document Type"
  • re-label "Base docname" to "Reference Document"
  • next schedule date should be on top. after submission that is the most important thing
  • add indicator ("Active" - green, "Expired" - grey, "Draft" - red)
  • delete image " erpnext/docs/assets/img/subscription/subscription_schedules.png"

Subscription
subscription

To Make subscription for invoice
subscription

To view recurring sales invoices
subscription_schedules

Dashboard View
screen shot 2017-07-26 at 8 30 14 pm

Indicator in the listview
screen shot 2017-08-29 at 3 52 06 pm

Fixed #6313

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 4 times, most recently from d1bce2b to 6aec08a Compare July 26, 2017 06:05
@GSLabIt
Copy link

GSLabIt commented Jul 26, 2017

@rohitwaghchaure is possible to add "Every 4 weeks" in Frequency? Seems the trend is moving in that direction ..

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 2 times, most recently from 44e0a5e to 6143f7b Compare July 26, 2017 06:43
@rmehta
Copy link
Member

rmehta commented Jul 26, 2017

  • I don't like the child table.
  • There must be option to make it directly from the Sales Invoice (Make -> Subscription)
  • patch?

@rohitwaghchaure
Copy link
Collaborator Author

rohitwaghchaure commented Jul 26, 2017

Fixed!
Removed child table from the subscription, added next_schedule_date field
Added provision to make subscription from the document
Added subscription in the dashboard of the document

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 3 times, most recently from 37bb8c3 to 746df7b Compare July 27, 2017 10:45
@rmehta
Copy link
Member

rmehta commented Jul 28, 2017

👍 for UI tests

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 9 times, most recently from 4ac438c to 0461ce9 Compare August 4, 2017 09:36
@mbauskar
Copy link
Contributor

mbauskar commented Aug 9, 2017

@rohitwaghchaure,

Please rebase and push again !!. Closing the PR for now

@mbauskar mbauskar closed this Aug 9, 2017
@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 2 times, most recently from f303588 to b8bcfb2 Compare August 11, 2017 10:06
"in_global_search": 0,
"in_list_view": 0,
"in_standard_filter": 0,
"label": "Subscription Detail",
Copy link
Member

Choose a reason for hiding this comment

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

just call this "Subscription"

@@ -0,0 +1,28 @@
Subscription module plays an important role in the every oraginization, specially for the organization who serve the services to the customers based on period.
Copy link
Member

Choose a reason for hiding this comment

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

remove this sentence


refresh: function(frm) {
if(frm.doc.docstatus == 1) {
let label = 'View ' + frm.doc.base_doctype;
Copy link
Member

Choose a reason for hiding this comment

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

should be translated! __('View {0}', [frm.doc.base_doctype])

let label = 'View ' + frm.doc.base_doctype;
frm.add_custom_button(__(label),
function() {
frm.trigger("view_subscription_document");
Copy link
Member

Choose a reason for hiding this comment

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

why new function, just add this inline?

'2': 'Cancelled'
}[cstr(self.docstatus or 0)]

self.db_set("status", status)
Copy link
Member

Choose a reason for hiding this comment

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

why db_set? this is called before saving in validate

make_subscription_entry()

docnames = frappe.get_all(doc.base_doctype, {'subscription': doc.name})
self.assertEquals(len(docnames), months)
Copy link
Member

Choose a reason for hiding this comment

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

check if values are copied over correctly!

@@ -128,13 +128,15 @@ def get_next_date(dt, mcount, day=None):

Copy link
Member

@rmehta rmehta Aug 28, 2017

Choose a reason for hiding this comment

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

rename this file to subscription

move this code to subscription.py

@rohitwaghchaure
Copy link
Collaborator Author

rohitwaghchaure commented Aug 29, 2017

@rmehta
Fixed!

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 2 times, most recently from f28a897 to c45b242 Compare August 29, 2017 10:50
@@ -3384,7 +3384,7 @@
"bold": 0,
"collapsible": 0,
"columns": 0,
"fieldname": "subscription",
"fieldname": "subscription_id",
Copy link
Member

Choose a reason for hiding this comment

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

noooo! Leave this as subscription link field naming is very important!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

@rohitwaghchaure rohitwaghchaure force-pushed the subscription_module branch 3 times, most recently from 68270ec to 0ff2cb5 Compare September 1, 2017 11:00
@rmehta
Copy link
Member

rmehta commented Sep 1, 2017

@rohitwaghchaure can you check codacy?

@rohitwaghchaure
Copy link
Collaborator Author

@rmehta Fixed codacy!

@nabinhait nabinhait merged commit 166b78f into frappe:develop Sep 7, 2017
tundebabzy pushed a commit to tundebabzy/erpnext that referenced this pull request Sep 8, 2017
* [Enhance] Subscription module

* Added view documents from the subscription form

* Test cases

* documentation

* UI Test cases, fixes

* Removed child table in the subscription

* Provision to make subscription from the document, added subscription in the dashboard for the sales and buying flow

* added patch to make subscription from the recurring data

* Rename field subscriptio to subscription_id, added new test cases, remove recurring_document from controller

* renamed subscription_id to subscription
@GSLabIt
Copy link

GSLabIt commented Sep 12, 2017

@rohitwaghchaure what about to create a subscription not "stand alone"? I mean create it not invoice based but "item based", so that would be possible to choose the items to bill for the subscription and create invoice based on that.

tundebabzy pushed a commit to tundebabzy/erpnext that referenced this pull request Oct 30, 2017
* [Enhance] Subscription module

* Added view documents from the subscription form

* Test cases

* documentation

* UI Test cases, fixes

* Removed child table in the subscription

* Provision to make subscription from the document, added subscription in the dashboard for the sales and buying flow

* added patch to make subscription from the recurring data

* Rename field subscriptio to subscription_id, added new test cases, remove recurring_document from controller

* renamed subscription_id to subscription
@hwinkel
Copy link

hwinkel commented Dec 14, 2019

@GSLabIt as following the subscription topic since 2013 in erpnext, I would also suggest to have this not Invoice based. As a monthly subscription for a Service i.e. SaaS/IaaS Service will include also usage based costs, and also may vary in the amount of booked Items. From looking at the subscription management docs and video today this seems no bee possible with the current implementation.

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.

Subscription functionality redesign
6 participants