[D8] Projects Management System [#2847546]

Synopsis

This project provide a complete enterprise project management solution, give you a flexible approach to defining and
managing your projects and the people, schedules, deliverables, and finances associated with them.

Requirements

business_menu, business_toolbar and other modules from business_core(https://www.drupal.org/sandbox/jianhe/2846751).
organization and other module from human_resources(https://www.drupal.org/sandbox/jianhe/2847047).

Roadmap

https://www.drupal.org/node/2846532

Dependencies

Drupal 8.x

Comments

Comment #1

28 January 2017 at 03:08

jian he created an issue. See original summary.

Comment #2

jian he CreditAttribution: jian he commented 28 January 2017 at 03:09

Status:

Active

» Needs review

Comment #3

PA robot CreditAttribution: PA robot commented 28 January 2017 at 05:23

Issue summary:	View changes
Status:	Needs review	» Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxjianhe2847056git

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Comment #4

jian he CreditAttribution: jian he commented 28 January 2017 at 07:22

Issue summary:	View changes
Status:	Needs work	» Needs review

Now the only issue on pareview.sh is: Visibility must be declared on method ...
These issue are all on XxxTest.php, but for drupal testing code standard, do not need visibility declare on testing functions.

Comment #5

PA robot CreditAttribution: PA robot commented 29 January 2017 at 11:23

Status:

Needs review

» Closed (duplicate)

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2847784

Project 2: https://www.drupal.org/node/2847546

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

I'm a robot and this is an automated message from Project Applications Scraper.

Comment #6

jian he CreditAttribution: jian he commented 29 January 2017 at 11:33

Status:

Closed (duplicate)

» Needs review

Yes there has two project: Projects Management System and Human Resources Management System.
Now I have make another issue to duplicated status, and make this one to review.

Comment #7

jeetendrakumar CreditAttribution: jeetendrakumar as a volunteer and at HyTechPro.com commented 20 February 2017 at 06:12

Status:

Needs review

» Needs work

Hello,

Please fix pareview.sh errors.

Review of the 8.x-2.x branch (commit 9dfd0d3):

./modules/project/project.install: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming

function project_update_8201() {
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.


FILE: /root/repos/pareviewsh/pareview_temp/CHANGELOG.txt
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

FILE: ...repos/pareviewsh/pareview_temp/modules/issue/src/Tests/IssueTest.php
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
42 | ERROR | [x] Visibility must be declared on method
| | "testProjectIssue"
62 | ERROR | [x] Visibility must be declared on method "testTaskIssue"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: .../pareview_temp/modules/deliverable/src/Tests/DeliverableTypeTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
38 | ERROR | [x] Visibility must be declared on method "testList"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...ewsh/pareview_temp/modules/deliverable/src/Tests/DeliverableTest.php
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
42 | ERROR | [x] Visibility must be declared on method
| | "testProjectDeliverable"
62 | ERROR | [x] Visibility must be declared on method
| | "testTaskDeliverable"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...pareview_temp/modules/workplan/src/Controller/WorkplanController.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
7 | WARNING | [x] Unused use statement
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...pareviewsh/pareview_temp/modules/workplan/src/Tests/WorkplanTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
38 | ERROR | [x] Visibility must be declared on method
| | "testProjectWorkplan"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...pos/pareviewsh/pareview_temp/modules/task/src/Tests/TaskTypeTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
38 | ERROR | [x] Visibility must be declared on method "testList"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...t/repos/pareviewsh/pareview_temp/modules/task/src/Tests/TaskTest.php
--------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------
42 | ERROR | [x] Visibility must be declared on method "testProjectTask"
62 | ERROR | [x] Visibility must be declared on method "testSubTasks"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...viewsh/pareview_temp/modules/task/src/Plugin/views/argument/Task.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
50 | ERROR | [x] Visibility must be declared on method "title"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: .../pareview_temp/modules/project/src/Plugin/views/argument/Project.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
50 | ERROR | [x] Visibility must be declared on method "title"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...s/pareviewsh/pareview_temp/modules/project/src/Tests/ProjectTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
41 | ERROR | [x] Visibility must be declared on method "testList"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: ...reviewsh/pareview_temp/modules/project/src/Tests/ProjectTypeTest.php
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
38 | ERROR | [x] Visibility must be declared on method "testList"
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------

FILE: /root/repos/pareviewsh/pareview_temp/README.txt
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
28 | WARNING | Line exceeds 80 characters; contains 84 characters
----------------------------------------------------------------------

Time: 402ms; Memory: 10Mb

Comment #8

jian he CreditAttribution: jian he commented 30 January 2017 at 15:15

Status:

Needs work

» Needs review

@jeetendrakumar, thanks for your preview.
Now all the issues reported by pareview.sh has been fixed except these:

./modules/task/task.module: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming

function task_entity_extra_field_info() {
function task_workplan_view(array &$build, EntityInterface $entity, EntityViewDisplayInterface $display, $view_mode) {

Task module is sub module of projects module, and "task_" is exactly the correct prefix for task module. It seems preview.sh miss reported.

./modules/project/project.install: all functions should be prefixed with your module/theme name to avoid name clashes. See https://www.drupal.org/node/318#naming

function project_update_8201() {

Project module is also a sub module of projects module, and "project_update_" is exactly the correct prefix for update function. This is also a preview.sh issue.

Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
FILE: /root/repos/pareviewsh/pareview_temp/modules/task/task.module
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
27 | WARNING | Format should be "* Implements hook_foo().", "*
| | Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "*
| | Implements hook_foo_BAR_ID_bar() for xyz-bar.html.twig.",
| | or "* Implements hook_foo_BAR_ID_bar() for
| | xyz-bar.tpl.php.".

And my code is:

/**
 * Implements hook_ENTITY_TYPE_view() for workplan entities.
 */
function task_workplan_view(array &$build, EntityInterface $entity, EntityViewDisplayInterface $display, $view_mode) {

task is my sub module name, and workplan is the entity type. I don't found any issue for my code. I guess it's the preview.sh miss reported it.

Comment #9

pdenooijer CreditAttribution: pdenooijer as a volunteer commented 31 January 2017 at 16:19

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation. Only found the Drupal 7 project PM.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

(*) Rename your submodules to something like pms_project etc, because deliverable, issue, project, task and workplan are to general and could clash with other module names.

Comment #10

pdenooijer CreditAttribution: pdenooijer as a volunteer commented 31 January 2017 at 16:19

Status:

Needs review

» Needs work

Comment #11

jian he CreditAttribution: jian he commented 1 February 2017 at 09:31

@pdenooijer, Thanks for your advice.

Coding style & Drupal API usage
(*) Rename your submodules to something like pms_project etc, because deliverable, issue, project, task and workplan are to general and could clash with other module names.

I agree with you. I will rename the module name into more meaningful name. And I have renamed the project module into organization_project already. Because the project module depend on the organization module, and the "project" is exactly an organization's project.
And I will rename other module names into meaningful name within next 24 hours.

Comment #12

jian he CreditAttribution: jian he commented 1 February 2017 at 14:05

I have reverted module name change commit maked by #9,#11.
This is because the "organization_project" name will make confuse. After making some discuss with my team, here is some conclusion:
1. The module and entity name is important for make business clear.
2. Module name such as "pms_project" will make confuse of business terms, and the config file with "pms_project.type.*" will also make confuse of business terms, and the "project" entity will not only used in PMS, and will used for other business solutions. Just like the organization in HRMS(Human Resources Management System), the "organization" entity and module is not only used in HRMS, it used by PMS(Projects Management System) and IMS(Inventory Management System) and other business solutions, hrs_organization will miss believe it's only HRS, but it's not.
3. All the business modules(such as organization, people, supplier, project, deliverable...) are sub-modules and in "Business" packages, all these entity name and module name has clear business definitions. If the user downloaded the module and want to install it, they will know what's the sub-module is, and will not make the module clash I think.

Sorry for my poor english skill, and I will open for any suggestion and ideas.

Comment #13

pdenooijer CreditAttribution: pdenooijer as a volunteer commented 1 February 2017 at 17:55

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation. Only found the Drupal 7 project PM.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

I see you have tests in your modules, very good :)!
To bad it not possible to rename your sub modules. I don't know if the admin have any problems with that. So I leave it up to them to decide on that matter.

Small request

Could you review my project application :)? For extra motivation, see this page why this could help your request!

Comment #14

pdenooijer CreditAttribution: pdenooijer as a volunteer commented 1 February 2017 at 17:56

Status:

Needs work

» Reviewed & tested by the community

Comment #15

TR CreditAttribution: TR commented 27 February 2017 at 02:12

@jian he: About #8, you are correct, your function naming IS correct - pareview is giving you a false error. I reported that a few days ago in #2854785: False error about function naming.

About #12 ...
Unfortunately, Drupal has a flat namespace for module names (and for entity names, and for plugin names). That's not ideal, but it's something that isn't going to change in the near future so we have to live with it and work around it to ensure we don't have accidental name conflicts. The accepted way to do this in Drupal is to prefix your sub-modules with the name of your project. The suggestion from #9 to use pms_ as a prefix is a very good suggestion.

As it stands now, you have sub-modules named "task" and "project", but those names are already in use by other modules. If someone tries to use one of those modules at the same time as yours then they will have major problems. It would be good to avoid those problems by naming your modules uniquely to begin with.

Also, how is testing, installing via drush, etc. supposed to know which "task" module is needed when you list "task" in your dependencies?

This naming is also very important for your entities. An entity with id="project" is certainly going to conflict with other entities on a web site - everybody can't be using the simplest word to name their entity or project, and "project" is already taken.

You do not have to use pms_, but you really, really, REALLY should pick a prefix and use that prefix for your modules and entities and plugins.

Comment #16

jian he CreditAttribution: jian he commented 27 February 2017 at 02:45

@TR, Thanks for your review.

I agree with you. I will change the sub modules name and entity name to give a prefix.

All the sub modules in the 'Projects' and 'Business core' are exactly the CBOs(Common Business Object). For example, the "Deliverable" sub module is a CBO, it used not only in "Projects Management" business solution, but will also used in other business solutions such as "Supplier Management".

So my suggest is put a "cbo_" prefix for all the sub modules for "Business core" and "Projects" and all my other Business solutions project. I will make the code change immediately if you agree with the "cbo_" prefix.

Comment #17

jian he CreditAttribution: jian he commented 27 February 2017 at 15:49

#15 has been fixed.
project, task from 'Projects' has been changed into cbo_project, cbo_task.
item, organization, resource from 'Business core' has been changed into cbo_item, cbo_organization, cbo_resource.

Comment #18

jian he CreditAttribution: jian he commented 13 March 2017 at 16:59

Status:

Reviewed & tested by the community

» Fixed

The project projects has been promoted to a full project: https://www.drupal.org/project/projects

Thanks all.

Comment #19

jeetendrakumar CreditAttribution: jeetendrakumar as a volunteer and at HyTechPro.com commented 14 March 2017 at 04:41

Comment #20

TR CreditAttribution: TR commented 21 March 2017 at 04:17

@jian he:
There has been a major change to the project application process. Please read https://www.drupal.org/drupalorg/blog/goodbye-project-applications-hello...

Yes, you now have git access, so does everyone - there is no longer an application process solely for git access. Your project is now automatically promoted to a full project, but you do NOT have security advisory coverage unless you leave this issue open and complete the process. This is not mandatory, but you should be aware of this major change!

Comment #21

klausi

he/him||they/them

German

🇦🇹 Vienna

CreditAttribution: klausi as a volunteer commented 21 March 2017 at 08:57

Status:

Fixed

» Needs review

So I assume this still needs review.

Comment #22

jian he CreditAttribution: jian he commented 21 March 2017 at 14:00

Thanks @TR and @klausi, Yes the project need have security advisory coverage, so need leave this issue open and complete the process.

Comment #23

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno as a volunteer commented 18 May 2018 at 14:52

Priority:

Normal

» Critical

To the reviewers: Please change back the priority to Normal after doing a review.

Comment #24

sleitner CreditAttribution: sleitner commented 8 August 2018 at 21:19

Priority:	Critical	» Normal
Status:	Needs review	» Needs work

Automated Review

Pareview details: https://pareview.sh/pareview/https-git.drupal.org-project-projects.git/r...

Review of the 8.x-3.x branch (commit b6297f5):

Your README.txt does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
DrupalPractice has found some issues with your code, but could be false positives.

This automated report was generated with PAReview.sh, your friendly project application review script.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template. See pareview

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

(*) add prefix to submodules deliverable, issue and workplan machine name and class/function names to prevent namespace collision
(*) dependenciy prefixes(drupal:) for example in cbo_project.info (https://www.drupal.org/docs/8/creating-custom-modules/let-drupal-8-know-...)
See pareview

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

Comment #25

sleitner CreditAttribution: sleitner commented 8 August 2018 at 21:20

Issue summary:

View changes

Comment #26

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno as a volunteer commented 10 August 2018 at 08:04

Issue summary:

View changes

Comment #27

apaderno

he/him

Italian

Brescia, 🇮🇹 🇪🇺

CreditAttribution: apaderno as a volunteer commented 30 October 2018 at 12:51

Status:

Needs work

» Closed (won't fix)

If you are still interested on being able to opt into security coverage for projects you create, please open a new application using a project for which the only commits (for the time required to set the application's status to Fixed) are from you.
Please don't open a new application if you aren't sure to have time to dedicate to the application, or it will be closed again as won't fix.

I am closing this application due to lack of activity.