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

feat: Virtual DocType #12121

Merged
merged 38 commits into from
Apr 2, 2021
Merged

feat: Virtual DocType #12121

merged 38 commits into from
Apr 2, 2021

Conversation

Thunderbottom
Copy link
Contributor

@Thunderbottom Thunderbottom commented Dec 23, 2020

Summary

Virtual DocType is a feature-extension for DocType which allows developers to create DocTypes with custom data sources and DocType controller. The purpose is to define custom DocTypes in the system without creating a table in the database, while utilizing the frontend, resource APIs, and roles and permissions from the framework.

These Virtual DocTypes function exactly like normal DocTypes in the frontend and are indistinguishable for the end-user, but gives more control to the developer over the DocType's data source. With this, the data source for a Virtual DocType can be anything: an external API, a secondary database, JSON or CSV files, etc. This enables the developers to plug-in database backends other than MariaDB and Postgres, and makes the Frappe Framework even more powerful!

Usage

Creating a Virtual DocType

To create a Virtual DocType, just select the Virtual DocType checkbox while creating the DocType:

image

Creating a Custom Controller

As an example, the following controller code uses a JSON file as the DocType datasource:

class test_virtual(Document):
	def db_insert(self):
		d = self.get_valid_dict(convert_dates_to_str=True)
		with open("data_file.json", "w+") as read_file:
			json.dump(d, read_file)

	def load_from_db(self):
		with open("data_file.json", "r") as read_file:
			d = json.load(read_file)
			super(Document, self).__init__(d)

	def db_update(self):
		d = self.get_valid_dict(convert_dates_to_str=True)
		with open("data_file.json", "w+") as read_file:
			json.dump(d, read_file)

	def get_list(self, args):
		with open("data_file.json", "r") as read_file:
			return [json.load(read_file)]

To integrate other datasources with the Virtual DocType, you will need to add controller methods defining the database access.

Outcome

  • The frontend for Virtual DocTypes remain unchanged:

image

  • All the /api/resource methods defined by the framework are compatible with Virtual DocTypes.

Documentation Link: frappe/frappe_docs#71

@Thunderbottom Thunderbottom requested a review from a team December 23, 2020 11:33
@ghost ghost removed their request for review December 23, 2020 11:33
@barredterra barredterra added add-docs New feature should be have an entry in documentation to increase the discoverability travis-failing labels Dec 23, 2020
@rmehta rmehta added the add-test-cases Add test case to validate fix or enhancement label Dec 23, 2020
@szufisher
Copy link
Contributor

szufisher commented Dec 24, 2020

more elegant solution than my doctype variant #7946

I am wondering whether it is possible to also combine the doctype variant feature into this one?

the idea is as following

  1. add base doctype field DocType, filter only non virtual doctype to be selectable
  2. in get_doc and get_list, for virtual doctype with base doctype assigned, call the get_list and get_doc on base doctype, apply the meta on the result.

@Thunderbottom
Copy link
Contributor Author

I am wondering whether it is possible to also combine the doctype variant feature into this one?

the idea is as following

  1. add base doctype field DocType, filter only non virtual doctype to be selectable

  2. in get_doc and get_list, for virtual doctype with base doctype assigned, call the get_list and get_doc on base doctype, apply the meta on the result.

You can probably do that with the current implementation of Virtual DocType. Since here you need to define your own DocType controller, you can use any existing DocType as your data source and limit the fields that you want your Virtual DocType to have within the controller itself.

Create virtual doctype only for non custom doctype
Create virtual doctype only if developer mode is set
@Thunderbottom
Copy link
Contributor Author

This PR is ready to be reviewed! @barredterra @surajshetty3416 :)

@szufisher
Copy link
Contributor

Still waiting this to be merged.

@surajshetty3416
Copy link
Member

@Thunderbottom you have to enable edit rights for maintainers so that we can push required changes.

@surajshetty3416 surajshetty3416 added the update-branch Reviewer does not have permission to update your branch. label Mar 17, 2021
@Thunderbottom
Copy link
Contributor Author

doesn't show the option for me anymore.

image

@surajshetty3416 surajshetty3416 self-assigned this Mar 18, 2021
@surajshetty3416 surajshetty3416 added tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR. and removed travis-failing labels Mar 18, 2021
Comment on lines 43 to 44
except frappe.DoesNotExistError:
return []
Copy link
Member

@surajshetty3416 surajshetty3416 Mar 18, 2021

Choose a reason for hiding this comment

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

Cannot rely on this, frappe.throw shows an error modal on the client-side which is why UI tests are failing. Why is this required? What was the issue with the previous handling?
Screenshot 2021-03-18 at 9 05 48 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

frappe.db.exists makes a db call, in case of virtual DocType we wont have a table so it will always return empty.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was not able to reproduce the error which you mentioned
contack

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you think is the best way to handle this issue?

Copy link
Member

Choose a reason for hiding this comment

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

Can we have a exists method for virtual doctype as well? frappe.db.exists can internally use that method to check if virtual document exists.

Copy link
Contributor

@shridarpatil shridarpatil Mar 31, 2021

Choose a reason for hiding this comment

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

I don't think its a correct way to handle. Example consider if virtual doctype's backend is an api then we might end up calling same api twice, first call to check if document exists and second call to get the data.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we set frappe.local.message_log to false here?

Copy link
Member

Choose a reason for hiding this comment

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

first call to check if document exists and second call to get the data.

I think that's fine... but it will help in other cases where we are just using frappe.db.exists for different validations.

Copy link
Contributor

Choose a reason for hiding this comment

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

frappe.db.exists internally uses frappe.db.get_value so I have added support for frappe.db.get_value in virtual doctype,

@surajshetty3416
Copy link
Member

LGTM

@surajshetty3416 surajshetty3416 merged commit cf60714 into frappe:develop Apr 2, 2021
@surajshetty3416
Copy link
Member

@shridarpatil the change might have created some issue for ERPNext patch

Fatal Python error: Cannot recover from stack overflow.
https://github.com/frappe/erpnext/pull/25006/checks?check_run_id=2252328630

Let me know if you are able to figure out and fix it.

@barredterra barredterra removed the update-branch Reviewer does not have permission to update your branch. label Apr 3, 2021
@Olawale1
Copy link

Olawale1 commented May 6, 2021

Hi @Thunderbottom

Thanks a lot for this great addition to Frappe. Could you please guide on the following points (and possibly include in documentation):

  1. Where exactly should the controller code be located and is it possible to use Server Scripts for this ?
  2. Please add a sample script that could be used if the source is an external database

I think this would be helpful for a lot of other users as well

Thank you kindly

@haolei
Copy link

haolei commented May 6, 2021

When this feature will be available ? which release will include this feature? I see this feature was merged, but the codes can't be found in any branch

@nameduser0

This comment was marked as abuse.

@nameduser0

This comment was marked as abuse.

@haolei
Copy link

haolei commented May 14, 2021

@casesolved-co-uk in v13.1.0, only DocType support virutal_doc feature, but list view, form will not work ( couting in list view, form detail)

@nameduser0

This comment was marked as abuse.

@Andriesvn
Copy link

For anyone still interested, Applying the following fixes works perfectly.
#13321 d54ecd4 baf4259
Unfortunately still no Report Count so you need to set Disable Count and Disable Sidebarstats on All Virtual Doctype lists

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
tests-failing Automated tests are failing. Please resolve if it is due to the changes in current PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.