In Storm, since it was a Drupal 6 based approach, all of the additional data for the various node types was stored in custom tables. This requires that custom form elements be added to the node edit form. The Field API will allow the use of widgets supplied by other modules, cutting down on the amount of code that must be developed for this module. It will also take care of storage and versioning. And provides built in views integration. On the developer side of things it will allow us to take advantage of some new Drupal 7 niceties like the EntityFieldQuery.

Sub-issues for this task:
#2041659: PMOrganization: Migrate to field_api based fields.
#2043997: PMProject: Migrate to field_api based fields.
#2044023: PMTask: Migrate to field_api based fields

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

juliangb’s picture

We should definitely do this.

A question for discussion, is which contrib fieldtype modules we should rely on for the fields - which again will increase the functionality that we will then have access to.

(I don't want to be defining any field types within PM - it shouldn't be necessary).

What do people think of:
- Addressfield (instead of a series of text fields for address)
- Entity Reference (or References, perhaps, for storing the links between Org->Project->Task, etc)

n8j1s’s picture

I love the entity reference module and think that we should definitely go that route.

I have used addressfield several times. It can be buggy, but is currently one of the best options. Most of the issues I run into with it involve the widget so they're not insurmountable. And it's entirely likely they will have a stable release before this project is ready for a stable release.

I like the built in list field type for statuses and categories. Or the Taxonomy field might work nicely here too.

Built in Link field for links.

Phone still has no Drupal 7 stable release, but it looks promising. I did run into a problem with it on a recent project though. (#1832442: Validation is successful, but format function returns an empty string)

D34dMan’s picture

we know what the advantages are cause of using Fields.

But what do we need to bring about these goodies in,

1. Need to replace all database calls to EntityFieldQuery?
2. Need to re-factor all the code accessing data to use EntityMetadataWrapper?
3. Delete all tables pm has now ( courtesy storm ) and provide migrate path D6 to D7 using Feeds instead of more or less a table renaming maybe.
4. Provide a upgrade path from D7(non-field-version) to Field version, OR give a warning that there wont be any ( discourage anybody to use PM.

------------

Something that could ease our effort.
we create features for all pm node types. and the core module will only contain codes that interact with these core-features.

For example.

A feature could be 'pmorganizationfeature' it will contain the content type definition.

pmorganization has strict dependency on pmorganizationfeature.

Then there could be another feature for reports.

This would shift lots of repetitive task to automated code ( features ). Which i believe would result in fewer bugs, faster development and smaller code-base to maintain.

The maintainers could concentrate more on what really matters. how the content-type interacts with each other.

Some of those interactions could again be pushed to "rules" and bundled as yet another feature.

This approach would also promote involvement of Webmasters and those who are not that comfortable to fiddle with code in the development of Pm.

------------------

Taking it a step further...

Another extreme development method would be that "PM" never assumes what content type it is going to work on.

For example:

consider the module pmtask.

it could be possible for pmtask module to recognize more than one content type as "task". The mapping of mandatory fields that pmtask needs like "assignee", "duration", "parent-task" etc could be mapped and stored in database (ctools will help you in this).

This method would give users the chance to use PM as an engine.

nuf said.

***

So do we need to use fields? and if yes, to what extend are the developers "prepared" to go for it. Its a huge task ahead, unless i missed something in Drupal which allows us to could keep all tables as it is, and change to field_api based pm.

And we definitely need few maintainers :)

dbt102’s picture

in #3 ... are you proposing that we incorporate additional dependencies into the PM Module on the Drupal "Features" and the "Rules" modules? Both of these have a user base of over 100K so they are obviously popular.

Do we need to layout a roadmap for this effort?

I'm more than willing to help out with testing aggressively as dev patches come together...

Not being a expert programmer thou, I'm not sure I follow the implications of what you are proposing D34dman.

Based on your effort thus far tho, I know you are awesome.

D34dMan’s picture

Here is a small preview of what would happen to pmorganization module.

The entity would be created in "GUI" (click click click in a web interface) and then maintainer would create a feature. A feature is nothing but a module created through an automation process basically.

Then it could be added into pm folder and pmorganization declare a dependency on the newly created feature.

Now this action would render lots of code useless in pmorganization. To give you an idea, i have created a patch.

Offcourse we would need to introduce new functions ( hooks ) to hook into update process or somthing similar. Those functions are necessarily the same extra functions we would write even if we decide to go "code the field api" way.

So less code to maintain, much better product i say.

D34dMan’s picture

@db102

In #3 ... are you proposing that we incorporate additional dependencies into the PM Module on the Drupal "Features" and the "Rules" modules? Both of these have a user base of over 100K so they are obviously popular.

Yes. and also maybe a dependency on Strongarm too. This would enable us to create features that could save pm's site variable defaults.

Do we need to layout a roadmap for this effort?

We need to brainstorm on this first. a lot lot lot, we could get even better idea from the community.

I'm more than willing to help out with testing aggressively as dev patches come together...

That would be awesome :), but we would have fewer and fewer bugs i believe. Less codebase to maintain, fewer bugs. This is assuming feature is well matured enough to create bug free code. Which i believe it is for such a simple use case.

we all are awesome :) cause we are contributing to Drupal in someway and most importantly, trying to make things which we ourself want to use.

dbt102’s picture

FileSize
81.39 KB

Loaded patch from #5, but not sure what you are trying to show. In PM dashboard, ORG view does not show existing ORGs, nor can you create new ORGs from the PM Dashboard (or anywhere else).

If I look at existing Content (see attached), I can see the existing ORGs are still there, but when trying to view them, errors indicate that it just kind of craps-out.

D34dMan’s picture

@db102
aaah! no, the patch was not supposed to be applied, a feature should accompany that patch. The "patch" in #5 is for reading. That shows you how many lines we could "delete" from pmorganization module. That is some 974 lines of code removed :D.

And db102, you should install Dreditor on your browser and see those red mania in the patch file :)

dbt102’s picture

Thanks for the Dreditor tip D34dman. I installed it and now looking at patch #5 using it. It looks like the amount of code simplification would be huge in moving PM forward.

D34dMan’s picture

Also moving to field_api based approach should be done in a separate branch. Since the versions would be incompatible and a different upgrade path would exist for the same. I am interested in taking up the co-maintainership of the field api branch.

The thought process that goes into such a path are...

1. Architectural difference is huge.
2. Upgrade paths are different cause of "1" above.
3. whatever changes that are required to be done to functions have more or less nothing to do with the D7 upgrade code that would be written for current branch.
4. The current branch has more or less the same features that are in D6 version of Storm. The Field api branch is going to be different. The support would be different, the approach of the website admin would be different, the approach of the developer would be different. Its going to so much different that it requires special place in itself.
5. This branch would presumably have more dependencies on other contrib modules like rules and features than the current branch.

dbt102’s picture

OK, if I understand correctly, Features and Field_api migration should go hand in hand. The reason for this is so that Features would be used to ease field_api migration.

So, what I'm hearing then is that the fastest way forward for PM may be to move this issue (https://drupal.org/node/1915320) out of Phase 3 of D7 roadmap into a 2.x version dev?

This I suspect would then move additional D7 roadmap issues out of the 1x version bin into the 2x version bin...

D34dMan’s picture

Took some time to create a new feature. I did some changes to avoid conflict with pmorganization name space and used pm_organization.

For a new install of PM the patch could be applied and you can enable the pmorganization module which has a new dependency on the module included in "features" zip file.

the attributes integration is pending though. Yet to figure out what would be the best way to go about it. One option that comes to my mind is https://drupal.org/project/variable.

D34dMan’s picture

FileSize
10.78 KB

Please use the zip file provided with this comment with patch in #12.

juliangb’s picture

@D34dMan, @dbt102, thanks for reigniting this conversation.

This change is something that I believe is really important for PM in order to move from the current situation of everything being very custom to a new ideal of integrated with other modules.

The way I had imagined this was to introduce a dependency on a few modules that define Field types (such as date, and perhaps entity reference), and use the field hooks to add these fields to our content types.

I'm not convinced yet by the need to wrap these up in features - whilst I think that can be useful in distributions and when recording a website's configuration in code, but for a module it could potentially add complexity that isn't required. Could you put a few more details about why you think this could be a good way to go?

In terms of upgrade paths, I'm keen that there is a continuous upgrade path from Project Management 7.x-1.x to whatever the new version is - the lack of an upgrade path would be a real pain for existing users, and I think would be relatively easy to implement. On the other hand though, this makes it easier because the upgrade path from Storm can be handled totally separately (see issues in Storm queue), and ongoing development will only need to think about upgrading from the latest version of Project Management.

Interested to hear your thoughts on developing this functionality in a separate branch. Personally I'm not keen on this (I tried it before in the Storm module), because moving to a separate branch splits the userbase and focus. By having a single branch, there is more attention on the latest code. To counter your thought on this being a major architectural change - yes, I agree, but I think the point of an alpha release is that that doesn't matter too much - users know that there may be changes.

Raphael Dürst’s picture

I have to agree with juliangb concerning features.
I don't think features are the way to go in modules.
I'd rather write the code for fields myself than using generated code from the features module.
This way, we have much more control over what id actually does.
Also, we don't need dependencies to even more modules.

D34dMan’s picture

@julliangb

Could you put a few more details about why you think this could be a good way to go?

Nothing more than i had mentioned in the comments so far, out of which less code to maintain and test is my favourite :) == faster development and more time spend on framework than on content CRUD operation implementation, including view.

***
@Raphael
I was suggesting feature module cause it seems to generate code that is much better than i could come up with. (Being an embedded engineer i had my paranoid days where i had to check the machine code generated by C compilers, cause over there a mistake could mean a burnt hardware with no backups :P)

So well if its a personal project, i would definitely follow the path i have suggested. It saves me time and debugging. But i agree it may not be the best for the "pm" project.

juliangb’s picture

Before I chime in a lot more I'm going to have a look at the sample code you posted so that I can talk at least vaguely intelligently about the options.

Any other thoughts on upgrade paths and branch choice?

dbt102’s picture

The patch in #12 applied cleanly, and I installed the features directory ( from .zip) into sites/all/modules directory. It was very instructional. Thank you D34dman for making that effort!

I'm not real excited about a multitude of dependencies on other contributed modules for PM either. I am very interested in seeing PM move forward very quickly toward a stable release and a full featured module. I'm afraid that by the time D8 is released, PM will just be getting to the point where it is stable and some of the cool features that should be there are just barely functional.

With D8 Views will be part of core. The PM-D7 dependency then goes away with PM-D8 since its in core.

I think if Features module is used to greatly expedite progress on PM, this could be a very good thing for PM and adding it in as a dependency could be very worthwhile. If this sets up PM for a quicker upgrade to D8 I think it could be something more valuable than say a dependency on the Address module.

I know its a slippery road to go down, when we start to add dependencies. We know a dependency on a particular module makes good sense when its getting assimilated into core. And, when a dependency has a huge number of installs (like Date) it seems to makes sense as well. I think though that drawing the line somewhere will not be so easy.

juliangb’s picture

On the dependencies and progress towards PM-D8, here are a few facts without opinions attached.

Drupal 8:
http://drupalreleasedate.com/ - At the time of writing, no date was available because the number of critical bugs in core is increasing.
http://drupal.org/core/release-cycle - Note that we have just entered the "Polish Phase".
http://drupal.stackexchange.com/questions/7209/which-contrib-modules-are... - See the lists marked "entirely" and "partially".

Potential dependencies:
https://drupal.org/project/usage/date - 329639 - partially in D8 core - for date fields in project/task/ticket/timetracking
https://drupal.org/project/usage/link - 218274 - fully in D8 core - for URL field in organization (not sure this is really required, though)
https://drupal.org/project/usage/entityreference - 76873 - fully in D8 core - for linking of organizations/projects/tasks/tickets/timetrackings/...
https://drupal.org/project/usage/addressfield - 55239 - for address of organization. Module is a key component of Drupal Commerce, so likely to remain supported.
https://drupal.org/project/usage/features - 133026 - for packaging of content types
https://drupal.org/project/usage/email - 100465 - fully in D8 core - for email field on person and organization (not sure this is really required on the organization, probably is on person unless we move to using the Drupal user instead).

Are there any other modules that we are even potentially considering?

For reference:
https://drupal.org/project/usage/pm - 235 users
https://drupal.org/project/usage/drupal - 906941 users

juliangb’s picture

I've had a think through these issues and here's a proposal for a way forward.

1. Features

Having reviewed the feature, actually I think the code that it produces is actually very similar to the code that we would write ourselves - the features module is really just putting a wrapper round this to allow easy import and export.

My proposal is that we stick to implementing the hooks ourselves in the existing pmXYZ modules (i.e. no dependency on features), but that doesn't stop us using features in the development stage to create code that we can edit.

I wanted to put the list above of possible dependencies to emphasise that these modules are much bigger than Project Management and so the impact on the user of requiring the dependency and the risk of delay on upgrade is minimal. However, let's challenge for each field whether we need to have a formal dependency or whether we could make do without - noone wants PM to require 10 other modules each time you install!

I suggest we start with the date fields as that will lead to the biggest simplification for PM (we have a lot of custom code that translates between timestamps and date objects etc) whilst also allowing easier integration when we put the Gantt chart functionality back in. The issue is #1929158: Use date fields from the date module.

We should then continue this discussion to focus on the next "hit", where conversion to Field API makes the biggest improvement in functionality and the biggest simplification in terms of our code.

2. Drupal 8 Upgrade

Whilst I know that the core maintainers are starting to call for modules to be upgraded, I think that (for now) we should deliberately avoid focussing on it - precisely so that we can keep momentum coming with the D7 version and get great new features in as well as these architecture changes.

There are two main reasons why I think the D7 to D8 upgrade will be simpler than the D6 to D7:

  • We have less code to port: The 7.x-1.x-dev tar.gz file is currently 190KB compared to 242KB for the 6.x-2.x-dev file (20% less).
  • The code is simpler: During the port from D6 to D7, we have reworked several ways that we do things, including using Views, removed custom javascript and other redundant code. What we have left will now be much more "standard" - so there will be more documentation to help us port the code quickly.

Some of the code in the D6 version really still felt like D5 style code - we've come a long way.

I realise that there are big changes from D7 to D8 core, but this will be the same for all modules and for PM we can link in to that momentum and use the community documentation to help us.

Thoughts?

D34dMan’s picture

Awesome!

So we have best of both worlds.

So is it safe to assume that this version of PM will "NOT" have any custom fields at all?

--------------
@juliangb

I suggest we start with the date fields as...

By starting you meant starting what: discussion or coding?

Am ok with discussion, but coding should be done per content type basis imo.

My Vote on Potential dependencies:

1. Date Field ( this is a no brainer, we just need it, absolutely truly madly deeply period).
2. Link ( not required )
3. Entity Reference (ok )
4. AddressField ( not required )
5. Email ( not required )

Reason for "not required", just for one content type which is a required one, creating 3 dependencies doesn't make sense.

Also those fields have no part to play in PM logic, so it can be optionally be downloaded and installed and used by those who need it.

Regarding Entity Reference, i had a look at other possible solution like relation which doesn't hold a single vote with respect to well matured Entity Reference.

Raphael Dürst’s picture

Generating code with Features and them modifying and implementing this code is a good idea.
This gets my vote too. ;)

And I agree that we definitely need the Date field, Entity Reference would be nice too.

dbt102’s picture

Great job @juliangb in stating the facts (#19) and drafting proposal (#20) !!!

I too agree with moving forward this way.

D34dMan’s picture

1. Date Field ( this is a no brainer, we just need it, absolutely truly madly deeply period).
2. Link ( not required )
3. Entity Reference (ok )
4. AddressField ( not required )
5. Email ( not required )

My addition to the list

6. Print module. ( For invoice and reports).

juliangb’s picture

By starting you meant starting what: discussion or coding?

I meant coding. Good point that we may wish to do this for a whole content type at once. But I'm also aware that there are common elements.

Perhaps we should start sharing a few patches for proof of concept, but discuss before committing any.

My Vote on Potential dependencies: ...

What are we suggesting happens with other fields - where the appropriate field type module is marked "not required" - would we just put a textfield in?

(This would effectively emulate our custom fields - but we'd lose features such as automatic validation etc).

6. Print module. ( For invoice and reports).

Could this be optional rather than a dependency? I think we'd only have to provide a print.css file and if the Print module was enabled it would print nodes to PDF for us.

juliangb’s picture

Thinking a bit more about doing this by content type, and how we could test the concepts in practice, in a logical order.

How about we try the "note" content type as a test run for this?

It is fairly simple set of fields (reference to other content types only) and has no "other logic". This would effectively mean starting with a new dependency on entity reference.

We could then move on to timetrackings - these would add date and checkboxes.

D34dMan’s picture

Assigned: Unassigned » D34dMan

I was just trying to create a patch, when i realized we could potentially reuse certain fields.

For example field_pmorganization_nid, field_pmtask_nid, field_pmticket_nid, field_pmperson_nid ...

come to think of it, shouldn't we needs some sort of naming convention for fields?

field_pmorganization_nid for fields referencing pmorganization. Which again could simply be field_pmorganization!

So, in case we are reusing fields, who would be declaring it?

Otherwise we can decide not to reuse the fields, and declare separate fields for every content type. This would mean a pmorganization reference would be stored in pmtask in field_pmtask_pmorganization.

So, should we declare separate fields for each content type?

juliangb’s picture

I'd personally be in favour of reusing fields.

It makes it much simpler when writing views, etc., and I think could mean that we potentially can have better search functionality where we look for all nodes flagged with a particular organization (for example).

I suggest the entity reference type fields are declared by the module that they reference (for example the organization module could declare the organization field, which is then added as an instance on several other content types).

Naming convention - sounds good - any suggestions?

By the way, once we start with patches, could they be in separate issues for clarity?

D34dMan’s picture

Assigned: D34dMan » Unassigned

@juliangb

I'd personally be in favour of reusing fields.

- So wouldn't it mean we would have to start with a content type that has no dependency on other content types. pmorganization would be the candidate.

Naming convention - sounds good - any suggestions?

For reference fields, it could be
field_[content type machine name]
examples, field_pmorganization, field_pmtask, field_pmperson, field_pmticket, field_pmproject...

For all others it could be of the format,
field_[content type machine name]_[storm_field_name]
examples, field_pmorganization_pricemode, field_pmorganization_www etc.
exception could be field_pmorganization_mail instead of field_pmorganization_email to be more consistent with Drupal's machine naming convention for email field.

The above naming convention would automatically create a name spaced field names i believe.

D34dMan’s picture

I think we have to drop the "field_" prefix that i had used in the above comment and make it absolutely name spaced ( field prefixed by the module name that is declaring it, the naming convention in #29 came from using "features module" to generate the code).

For reference fields, it could be
[content type machine name]_id
examples: pmorganization_id, pmtask_id, pmperson_id, pmticket_id, pmproject_id...

For all others it could be of the format,
[content type machine name]_[storm_field_name]
examples: pmorganization_pricemode, pmorganization_www etc.
exception could be made in certain cases like the below after a discussion.
For pmorganization content type.
pmorganization_mail instead of pmorganization_email to be more consistent with Drupal's machine naming convention for email field.
orglanguage to pmorganization_language.

Raphael Dürst’s picture

Issue summary: View changes

Added list of issues

Raphael Dürst’s picture

Issue summary: View changes

Updated issue summary.

juliangb’s picture

Issue summary: View changes

Ref meta issue for merge.

juliangb’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Status: Active » Fixed

Done (near enough). Remaining tasks will be created as separate issues.

Håvard’s picture

Just for clarification, will it be possible to upgrade from 7.x-1.x to 7.x-2.x?

Thanks a lot for the PM modules, been using them in several projects this year. I think PM has the best approach among the project modules/distros. I really hope that I don't have to manually upgrade the fields.....?

Håvard’s picture

Ooops! Displaying the files again...

dbt102’s picture

Nice job guys!!!

juliangb’s picture

Yes, there is a full upgrade path from 1.x - 2.x.

Just note, due to some of the changes (we now use user accounts instead of PM Person nodes) some manual intervention may be required if data is inconsistent in your installation. PM will guide you through fixing this if required. Also, note the PM Invoice module will be a separate download in 2.x.

Håvard’s picture

Thanks for fast reply :-)

I'm not sure how these upgrades paths works in general and this is perhaps off-topic. However, my PM-sites is getting pretty big now, and the clients would benefit a lot on the API Fields, so I 'm kind of stressed out by this upgrade path. Is there any documentation showing the process?

juliangb’s picture

The general principle of it is that you can simply replace the old version of the project (1.x) with the new version (2.x), then navigate to update.php. This will run the upgrade code - then simply follow the on screen instructions.

I have been testing the upgrade path on a site with almost 700 PM nodes, where the migration takes approx 10mins.

I would suggest that you try this on a copy of your site first.

Håvard’s picture

@juliangb - you are amazing :-)

Håvard’s picture

I got the following error when running update.php. Any ideas?

Warning: array_map() expects parameter 1 to be a valid callback, no array or string given in _pmperson_migrate_get_conflicting_email_with_attached_users_table() (line 265 of /opt/lampp/htdocs/multisite_core/sites/all/modules/pm/pmperson/includes/pmperson.migrate.inc).

Edit: Created an issue here

D34dMan’s picture

Hi Håvard,

Could you please create a new issue regardin the issue in #40. It would be easier to follow it. And as much details as possible please. If you have database logging turned on, you might have a look in there for extra errors reported there which could be useful.

Also do mention from which version you where trying to migrate. Thank you.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.