[16.0][ADD] project_task_pull_request_state #1189

Volodiay622 · 2023-11-06T19:00:24Z

It adds a "State" field to Task alongside with PR URI field.

ivs-cetmix · 2023-11-07T18:02:08Z

project_task_pull_request_state/models/project_task.py

+        ]
+
+    @api.model
+    def create(self, vals):


Use multi record "create" implementation with vals_list instead of vale and api.create multi decorator

project_task_pull_request_state/models/project_task.py

ivs-cetmix · 2023-11-07T18:06:42Z

project_task_pull_request_state/models/project_task.py

+                    pr_state = ICPSudo.get_param(
+                        "project_task_pull_request_state.pr_state_default"
+                    )
+                    task.pr_state = pr_state


What if no config value is defined or it is empty?

ivs-cetmix · 2023-11-14T15:12:01Z

pandoc-3.1.9-1-amd64.deb

What is is file doing here?) Is it needed? Please remove if not

ivs-cetmix · 2023-11-14T15:17:05Z

project_task_pull_request_state/readme/CONTRIBUTORS.md

@@ -0,0 +1 @@
+- Cetmix <https://cetmix.com/>


Please use MD format for links

ivs-cetmix · 2023-11-14T15:18:09Z

project_task_pull_request_state/tests/test_project_task_pull_request_state.py

+class TestPullRequestState(TransactionCase):
+    @classmethod
+    def setUpClass(cls):
+        super(TestPullRequestState, cls).setUpClass()


Disable tracking unless you need it for functional purposes

@classmethod def setUpClass(cls): super().setUpClass() cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))

ivs-cetmix · 2023-11-25T16:32:21Z

project_task_pull_request_state/models/project_task.py

+
+    pr_state = fields.Selection(
+        selection=lambda self: self._selection_pr_state(),
+        track_visibility="onchange",


This format of the "track_visibility" is not used any longer. Actually the is a warning in tests for that.
Here is a reference for that.

ivs-cetmix · 2023-11-25T16:43:41Z

project_task_pull_request_state/models/res_config_settings.py

+        "PR URI is added to a task in this project",
+    )
+
+    def set_values(self):


This function is not needed. Please use config_parameter in the field definition instead

pr_state_default = fields.Selection( selection=lambda self: self.env["project.task"]._selection_pr_state(), string="Default PR State", config_parameter="project_task_pull_request_state.pr_state_default" help="Default PR state that will be set when " "PR URI is added to a task in this project", )

ivs-cetmix · 2023-11-25T16:45:30Z

project_task_pull_request_state/readme/CONTRIBUTORS.md

@@ -0,0 +1 @@
+- Cetmix <@cetmix.com>


Please check back to
Cetmix <https://cetmix.com/>

project_task_pull_request_state/__manifest__.py

project_task_pull_request_state/models/project_task.py

ivs-cetmix · 2023-12-01T15:38:24Z

project_task_pull_request_state/models/project_task.py

+        new_recs = super(ProjectTaskState, self).create(vals_list)
+
+        # Set pr state if URI is provided at creation
+        for new_rec, vals in zip(new_recs, vals_list):


pr_state will be computed automatically upon creation. No need to call this function explicitly

project_task_pull_request_state/models/project_task.py

Volodiay622 · 2023-12-01T20:15:24Z

project_task_pull_request_state/models/project_task.py

@@ -17,14 +17,6 @@ class ProjectTaskState(models.Model):
        readonly=False,
    )

-    _sql_constraints = [


I'm not sure if this is necessary. But I also don't understand why it doesn't work, maybe you have some idea. Now it's occured this error

Bearnard21

Functionality OK

ivs-cetmix · 2023-12-15T20:24:03Z

project_task_pull_request_state/readme/CONTRIBUTORS.rst

@@ -0,0 +1 @@
+* Cetmix <https://cetmix.com/>


Please complete this section according to the latest updates in the DevMemo

Cetmix <https://cetmix.com/> Ivan Sokolov Vladimir Kalmykov

I'd suggest going with

* `Cetmix <https://cetmix.com/>`__ * Ivan Sokolov * Vladimir Kalmykov

Formatting, note the extra newline

ivs-cetmix

Code review LGTM

ivs-cetmix · 2023-12-30T18:08:59Z

hey @OCA/project-service-maintainers happy 2024! What a great opportunity to have one more PR merged while we are still in 2023 😄

OCA-git-bot · 2023-12-30T19:00:01Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

alexey-pelykh

Thanks for the PR! The comments are non-blocking

alexey-pelykh · 2024-01-03T14:13:30Z

project_task_pull_request_state/__init__.py

@@ -0,0 +1,3 @@
+# Copyright Cetmix OU 2023


init.py don't typically include copyright notice since there's nothing to apply copyright to.

alexey-pelykh · 2024-01-03T14:15:07Z

project_task_pull_request_state/models/project_project.py

+from odoo import fields, models
+
+
+class ProjectState(models.Model):


Model name follows the filename commonly, and since this model is a well-known one there's no point in having different name.

alexey-pelykh · 2024-01-03T14:16:31Z

project_task_pull_request_state/models/project_task.py

+        readonly=False,
+    )
+
+    def _selection_pr_state(self):


Do you anticipate that this could be plausibly extended in some cases? Please elaborate

alexey-pelykh · 2024-01-03T14:20:47Z

project_task_pull_request_state/readme/CONTRIBUTORS.rst

@@ -0,0 +1 @@
+* Cetmix <https://cetmix.com/>


I'd suggest going with

* `Cetmix <https://cetmix.com/>`__ * Ivan Sokolov * Vladimir Kalmykov

Formatting, note the extra newline

alexey-pelykh · 2024-01-03T14:21:49Z

project_task_pull_request_state/tests/test_project_task_pull_request_state.py

+        # Change pr_state
+        tasks.write({"pr_state": "closed"})
+
+        self.assertEqual(self.task_1.pr_state, "closed", "PR state must be 'closed")


I guess there's closing ' missing? If that message is even worth having

leemannd · 2024-01-18T10:07:28Z

project_task_pull_request_state/models/project_task.py

+from odoo import api, fields, models
+
+
+class ProjectTaskState(models.Model):


Suggested change

class ProjectTaskState(models.Model):

class ProjectTask(models.Model):

It would be better to follow the common naming as in https://github.com/OCA/project/pull/1189/files#r1440500916

@leemannd, thank you for suggestion. I added this to the last commit.

leemannd · 2024-01-22T08:45:50Z

/ocabot merge nobump

OCA-git-bot · 2024-01-22T08:46:06Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1189-by-leemannd-bump-nobump, awaiting test results.

Signed-off-by leemannd

OCA-git-bot · 2024-01-22T08:52:31Z

@leemannd The merge process could not be finalized, because command twine check odoo_addon_project_task_pull_request_state-16.0.1.0.0.2-py3-none-any.whl failed with output:

Checking 
odoo_addon_project_task_pull_request_state-16.0.1.0.0.2-py3-none-any.whl: �[31mFAILED�[0m
�[31mERROR   �[0m `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         line 55: Warning: Enumerated list ends without a blank line; unexpected
         unindent.                                                              
�[33mWARNING �[0m `long_description_content_type` missing. defaulting to `text/x-rst`.

ivs-cetmix · 2024-01-22T09:03:36Z

ocabot merge nobump

Hi @leemannd could you please check what could be a reason of the merge issue?

ivs-cetmix · 2024-02-04T21:28:27Z

Hi @leemannd I have rebase the branch, could you please try merging it again?

OCA-git-bot · 2024-02-06T10:23:12Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1189-by-leemannd-bump-nobump, awaiting test results.

Signed-off-by leemannd

OCA-git-bot · 2024-02-06T10:29:46Z

@leemannd The merge process could not be finalized, because command twine check odoo_addon_project_task_pull_request_state-16.0.1.0.0.1-py3-none-any.whl failed with output:

Checking 
odoo_addon_project_task_pull_request_state-16.0.1.0.0.1-py3-none-any.whl: �[31mFAILED�[0m
�[31mERROR   �[0m `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         line 55: Warning: Enumerated list ends without a blank line; unexpected
         unindent.                                                              
�[33mWARNING �[0m `long_description_content_type` missing. defaulting to `text/x-rst`.

sbidoul · 2024-02-06T11:52:08Z

That's a problem in the readme.

ivs-cetmix · 2024-02-06T11:53:24Z

That's a problem in the readme.

Thank you @sbidoul ! Any hint on how this should be solved?

sbidoul · 2024-02-06T11:55:54Z

Warning: Enumerated list ends without a blank line; unexpected unindent.

Look for a list that does not end with a blank line.

project_task_pull_request_state/readme/CONFIGURE.rst

ivs-cetmix · 2024-02-07T10:34:23Z

Warning: Enumerated list ends without a blank line; unexpected unindent.

Look for a list that does not end with a blank line.

thank you @sbidoul @yajo let me check that!

ivs-cetmix · 2024-02-07T18:52:39Z

Ok @leemannd let's give it another try 😆

leemannd · 2024-02-13T08:42:03Z

/ocabot merge nobump

OCA-git-bot · 2024-02-13T08:42:17Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-1189-by-leemannd-bump-nobump, awaiting test results.

OCA-git-bot · 2024-02-13T08:48:50Z

Congratulations, your PR was merged at 874e53a. Thanks a lot for contributing to OCA. ❤️

Volodiay622 marked this pull request as draft November 6, 2023 19:01

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 21886e2 to 93c8ef7 Compare November 6, 2023 19:07

Volodiay622 marked this pull request as ready for review November 6, 2023 22:58

ivs-cetmix suggested changes Nov 7, 2023

View reviewed changes

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 12772fe to 1192515 Compare November 7, 2023 20:59

Volodiay622 marked this pull request as draft November 7, 2023 21:16

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 9fe5e9e to f754cee Compare November 7, 2023 21:20

Volodiay622 marked this pull request as ready for review November 7, 2023 21:20

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from f754cee to 0bf3814 Compare November 13, 2023 03:21

ivs-cetmix suggested changes Nov 14, 2023

View reviewed changes

ivs-cetmix suggested changes Nov 25, 2023

View reviewed changes

Volodiay622 marked this pull request as draft November 30, 2023 16:29

Volodiay622 marked this pull request as ready for review November 30, 2023 17:11

ivs-cetmix suggested changes Dec 1, 2023

View reviewed changes

Volodiay622 commented Dec 1, 2023

View reviewed changes

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 5947b82 to af27f0b Compare December 4, 2023 02:00

Bearnard21 approved these changes Dec 8, 2023

View reviewed changes

ivs-cetmix suggested changes Dec 15, 2023

View reviewed changes

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch 2 times, most recently from 801719b to 120a008 Compare December 15, 2023 21:46

ivs-cetmix approved these changes Dec 15, 2023

View reviewed changes

OCA-git-bot added the approved label Dec 15, 2023

Volodiay622 force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 120a008 to 3616588 Compare December 29, 2023 18:27

OCA-git-bot added the ready to merge label Dec 30, 2023

alexey-pelykh reviewed Jan 3, 2024

View reviewed changes

leemannd reviewed Jan 18, 2024

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Jan 22, 2024

Merge PR #1189 into 16.0

39dbec9

Signed-off-by leemannd

OCA-git-bot added the bot is merging ⏳ label Jan 22, 2024

OCA-git-bot removed the bot is merging ⏳ label Jan 22, 2024

ivs-cetmix force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch 2 times, most recently from a0d4a09 to 03f3149 Compare February 4, 2024 21:26

OCA-git-bot added a commit that referenced this pull request Feb 6, 2024

Merge PR #1189 into 16.0

592af83

Signed-off-by leemannd

OCA-git-bot added the bot is merging ⏳ label Feb 6, 2024

OCA-git-bot removed the bot is merging ⏳ label Feb 6, 2024

yajo reviewed Feb 7, 2024

View reviewed changes

project_task_pull_request_state/readme/CONFIGURE.rst Outdated Show resolved Hide resolved

[IMP] project_task_pull_request_state: add module

69e67df

ivs-cetmix force-pushed the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch from 2a44d74 to 69e67df Compare February 7, 2024 18:31

OCA-git-bot added the bot is merging ⏳ label Feb 13, 2024

OCA-git-bot merged commit b232e80 into OCA:16.0 Feb 13, 2024
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Feb 13, 2024

ivs-cetmix deleted the 16.0-t2959-project_task_pull_request_state-migration_from_11.0 branch February 13, 2024 09:54

		from odoo import fields, models


		class ProjectState(models.Model):

		from odoo import api, fields, models


		class ProjectTaskState(models.Model):

	class ProjectTaskState(models.Model):
	class ProjectTask(models.Model):

[16.0][ADD] project_task_pull_request_state #1189

[16.0][ADD] project_task_pull_request_state #1189

Conversation

Volodiay622 commented Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bearnard21 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ivs-cetmix left a comment

Choose a reason for hiding this comment

ivs-cetmix commented Dec 30, 2023

OCA-git-bot commented Dec 30, 2023

alexey-pelykh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leemannd commented Jan 22, 2024

OCA-git-bot commented Jan 22, 2024

OCA-git-bot commented Jan 22, 2024

ivs-cetmix commented Jan 22, 2024

ivs-cetmix commented Feb 4, 2024

OCA-git-bot commented Feb 6, 2024

OCA-git-bot commented Feb 6, 2024

sbidoul commented Feb 6, 2024

ivs-cetmix commented Feb 6, 2024

sbidoul commented Feb 6, 2024

ivs-cetmix commented Feb 7, 2024

ivs-cetmix commented Feb 7, 2024

leemannd commented Feb 13, 2024

OCA-git-bot commented Feb 13, 2024

OCA-git-bot commented Feb 13, 2024

Volodiay622 commented Nov 6, 2023 •

edited

Loading