-
Notifications
You must be signed in to change notification settings - Fork 1
Pull request for issue #8 #9
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| from . import models |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # -*- coding: utf-8 -*- | ||
| { | ||
| 'name': 'ODF Data Quality Dashboard', | ||
| 'version': '18.0.1.0.0', | ||
| 'summary': 'Data Quality Dashboard for Odoo', | ||
| 'author': 'Odoo Community Association (OCA), Odoo Data Flow', | ||
| 'website': 'https://github.com/OCA/odoo-data-flow', | ||
| 'license': 'AGPL-3', | ||
| 'category': 'Tools', | ||
| 'depends': ['base'], | ||
| 'data': [ | ||
| 'security/ir.model.access.csv', | ||
| 'views/data_quality_issue_views.xml', | ||
| 'views/menus.xml', | ||
| 'data/scheduled_actions.xml', | ||
| ], | ||
| 'installable': True, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <odoo> | ||
| <data noupdate="1"> | ||
|
|
||
| <record id="ir_cron_run_data_quality_checks" model="ir.cron"> | ||
| <field name="name">Data Quality: Run Checks</field> | ||
| <field name="model_id" ref="model_odf_data_quality_issue"/> | ||
| <field name="state">code</field> | ||
| <field name="code">model._run_data_quality_checks()</field> | ||
| <field name="user_id" ref="base.user_root"/> | ||
| <field name="interval_number">1</field> | ||
| <field name="interval_type">days</field> | ||
| <field name="numbercall">-1</field> | ||
| <field name="doall" eval="False"/> | ||
| </record> | ||
|
|
||
| </data> | ||
| </odoo> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| from . import data_quality_issue |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # -*- coding: utf-8 -*- | ||
| from odoo import api, fields, models | ||
| from odoo.addons.base.models.res_partner import Partner | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
|
|
||
| class DataQualityIssue(models.Model): | ||
| _name = 'odf.data.quality.issue' | ||
| _description = 'Data Quality Issue' | ||
|
|
||
| name = fields.Char( | ||
| string='Name', | ||
| required=True, | ||
| ) | ||
| issue_type = fields.Char( | ||
| string='Issue Type', | ||
| ) | ||
| related_record = fields.Reference( | ||
| selection=[ | ||
| ('res.partner', 'Partner'), | ||
| ('product.product', 'Product'), | ||
| ], | ||
| string='Related Record', | ||
| ) | ||
| status = fields.Selection( | ||
| selection=[ | ||
| ('new', 'New'), | ||
| ('in_progress', 'In Progress'), | ||
| ('resolved', 'Resolved'), | ||
| ], | ||
| string='Status', | ||
| default='new', | ||
| ) | ||
| notes = fields.Text( | ||
| string='Notes', | ||
| ) | ||
|
|
||
| @api.model | ||
| def _run_data_quality_checks(self): | ||
| self._check_partner_vat() | ||
|
|
||
| @api.model | ||
| def _check_partner_vat(self): | ||
| """Check for partners with invalid VAT.""" | ||
| from datetime import timedelta | ||
| yesterday = fields.Datetime.now() - timedelta(days=1) | ||
| partners_to_check = self.env['res.partner'].search([ | ||
| ('write_date', '>=', fields.Datetime.to_string(yesterday)), | ||
| ('vat', '!=', False), | ||
| ('vat', '!=', ''), | ||
| ]) | ||
| for partner in partners_to_check: | ||
| # This is a placeholder for a real VAT validation. | ||
| # For this example, we consider a VAT invalid if it has less than 3 chars. | ||
| if len(partner.vat) < 3: | ||
| # Avoid creating duplicate issues for the same partner. | ||
| existing_issue = self.search_count([ | ||
| ('related_record', '=', f'res.partner,{partner.id}'), | ||
| ('issue_type', '=', 'Invalid VAT'), | ||
| ('status', '!=', 'resolved'), | ||
| ]) | ||
| if not existing_issue: | ||
| self.create({ | ||
| 'name': f"Invalid VAT number for '{partner.display_name}'", | ||
| 'issue_type': 'Invalid VAT', | ||
| 'related_record': f'res.partner,{partner.id}', | ||
| }) | ||
|
Comment on lines
+51
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current implementation iterates over partners and performs a database query ( # This is a placeholder for a real VAT validation.
# For this example, we consider a VAT invalid if it has less than 3 chars.
invalid_partners = partners_to_check.filtered(lambda p: p.vat and len(p.vat) < 3)
if not invalid_partners:
return
# Avoid creating duplicate issues for the same partner.
existing_issues = self.search_read(
[
('related_record', 'in', [f'res.partner,{p.id}' for p in invalid_partners]),
('issue_type', '=', 'Invalid VAT'),
('status', '!=', 'resolved'),
],
['related_record']
)
partners_with_issues = {issue['related_record'] for issue in existing_issues}
issues_to_create = []
for partner in invalid_partners:
partner_ref = f'res.partner,{partner.id}'
if partner_ref not in partners_with_issues:
issues_to_create.append({
'name': f"Invalid VAT number for '{partner.display_name}'",
'issue_type': 'Invalid VAT',
'related_record': partner_ref,
})
if issues_to_create:
self.create(issues_to_create) |
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,2 @@ | ||||||||||
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | ||||||||||
| access_odf_data_quality_issue_user,odf.data.quality.issue.user,model_odf_data_quality_issue,base.group_user,1,1,1,1 | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The access control list grants full CRUD (Create, Read, Update, Delete) permissions to all internal users (
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. /gemini propose a fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Certainly! To address the overly permissive access control, I propose introducing more granular permissions by defining two new security groups: a 'Data Quality User' with read-only access and a 'Data Quality Manager' with full CRUD permissions. Here's the suggested change for This change assumes that the new groups Example <?xml version="1.0" encoding="utf-8"?>
<odoo>
<data noupdate="0">
<record id="group_data_quality_user" model="res.groups">
<field name="name">Data Quality / User</field>
<field name="category_id" ref="base.module_category_extra_tools"/>
<field name="implied_ids" eval="[(4, ref('base.group_user'))]"/>
</record>
<record id="group_data_quality_manager" model="res.groups">
<field name="name">Data Quality / Manager</field>
<field name="category_id" ref="base.module_category_extra_tools"/>
<field name="implied_ids" eval="[(4, ref('group_data_quality_user'))]"/>
</record>
</data>
</odoo>Additionally, you would need to update 'data': [
'security/data_quality_groups.xml',
'security/ir.model.access.csv',
'views/data_quality_issue_views.xml',
'views/menus.xml',
'data/scheduled_actions.xml',
],This approach ensures that access to data quality issues is controlled based on user roles, improving the security and maintainability of the module.
Suggested change
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <odoo> | ||
| <data> | ||
|
|
||
| <!-- Form View --> | ||
| <record id="view_odf_data_quality_issue_form" model="ir.ui.view"> | ||
| <field name="name">odf.data.quality.issue.form</field> | ||
| <field name="model">odf.data.quality.issue</field> | ||
| <field name="arch" type="xml"> | ||
| <form string="Data Quality Issue"> | ||
| <sheet> | ||
| <group> | ||
| <field name="name"/> | ||
| <field name="issue_type"/> | ||
| <field name="related_record"/> | ||
| <field name="status"/> | ||
| </group> | ||
| <notebook> | ||
| <page string="Notes"> | ||
| <field name="notes"/> | ||
| </page> | ||
| </notebook> | ||
| </sheet> | ||
| </form> | ||
| </field> | ||
| </record> | ||
|
|
||
| <!-- Kanban View --> | ||
| <record id="view_odf_data_quality_issue_kanban" model="ir.ui.view"> | ||
| <field name="name">odf.data.quality.issue.kanban</field> | ||
| <field name="model">odf.data.quality.issue</field> | ||
| <field name="arch" type="xml"> | ||
| <kanban default_group_by="status"> | ||
| <field name="status"/> | ||
| <templates> | ||
| <t t-name="kanban-box"> | ||
| <div class="oe_kanban_global_click"> | ||
| <div class="oe_kanban_details"> | ||
| <strong><field name="name"/></strong> | ||
| <div>Type: <field name="issue_type"/></div> | ||
| <div>Record: <field name="related_record"/></div> | ||
| </div> | ||
| </div> | ||
| </t> | ||
| </templates> | ||
| </kanban> | ||
| </field> | ||
| </record> | ||
|
|
||
| <!-- Tree View --> | ||
| <record id="view_odf_data_quality_issue_tree" model="ir.ui.view"> | ||
| <field name="name">odf.data.quality.issue.tree</field> | ||
| <field name="model">odf.data.quality.issue</field> | ||
| <field name="arch" type="xml"> | ||
| <tree string="Data Quality Issues"> | ||
| <field name="name"/> | ||
| <field name="issue_type"/> | ||
| <field name="related_record"/> | ||
| <field name="status"/> | ||
| </tree> | ||
| </field> | ||
| </record> | ||
|
|
||
| <!-- Action --> | ||
| <record id="action_odf_data_quality_issue" model="ir.actions.act_window"> | ||
| <field name="name">Data Quality Issues</field> | ||
| <field name="res_model">odf.data.quality.issue</field> | ||
| <field name="view_mode">kanban,tree,form</field> | ||
| </record> | ||
|
|
||
| </data> | ||
| </odoo> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <odoo> | ||
| <data> | ||
|
|
||
| <!-- Top menu --> | ||
| <menuitem | ||
| id="menu_odf_root" | ||
| name="Data Flow" | ||
| sequence="10"/> | ||
|
|
||
| <!-- Sub menu --> | ||
| <menuitem | ||
| id="menu_odf_data_quality_dashboard" | ||
| name="Data Quality Dashboard" | ||
| parent="menu_odf_root" | ||
| action="action_odf_data_quality_issue" | ||
| sequence="10"/> | ||
|
|
||
| </data> | ||
| </odoo> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scheduled action is configured to run as the
rootuser (base.user_root), which poses a security risk by granting it the highest level of permissions. It is a best practice to follow the principle of least privilege and run scheduled actions with a dedicated user that has only the necessary permissions for the task.