Hi there,

First off thank you for making such a fantastic module. My colleagues and I have created some extra functionality and I have attached a patch for your review.

Added functionality:
Extra option on "Content Complete" home page titled "Percentages", where once a user has chosen some fields, they can apply a specific percentage to each field (in case they want to control certain fields having a higher/lower percentage etc.). There is also a check when the page is updated to inform the user if they are below/above 100% (and by how much).

I'm fairly new to Drupal but could you please take a look at the patch and, if you like the added functionality, implement it into the core module?

Many thanks,

Marc

P.S - Please note for anyone running 6.x-1.0 that you can also use this patch as it works for both releases. Any and all comments/suggestions most welcomed! :)

CommentFileSizeAuthor
#13 Content Complete - contradiction in percentages89.25 KBpvhee
#11 content_complete_percentages_6.x-1.x-dev_5.patch30.58 KBmarc.groth
#10 content_complete_percentages_6.x-1.x-dev_4.patch17.55 KBmarc.groth
#9 content_complete_percentages_6.x-1.x-dev_3.patch17.83 KBmarc.groth
#5 content_complete_percentages_6.x-1.x-dev_2.patch15.8 KBmarc.groth
content_complete_percentages_6.x-1.x-dev.patch15.61 KBmarc.groth

Comments

marc.groth’s picture

marc.groth commented
Status: Needs review » Patch (to be ported)
marc.groth’s picture

marc.groth commented
Status: Patch (to be ported) » Needs review
pvhee’s picture

pvhee commented

Thanks Marc for doing the effort of implementing this very nice functionality! I will have a look at the code ASAP to see how it can be included in the core module. To all, please report all tests/reviews of the patch in this issue.

marc.groth’s picture

marc.groth commented

Great thanks so much for the super fast response!!

As you can see I'm a slight newbie (two status changes, wasn't sure which was correct hehe).

Look forward to seeing your thoughts/comments/suggestions!! :)

marc.groth’s picture

marc.groth commented
StatusFileSize
new content_complete_percentages_6.x-1.x-dev_2.patch15.8 KB

Hi there,

We noticed a few bugs and have fixed them... Please use the updated patch (attached) and report back any problems etc. as normal.

Thanks very much,

Marc

pvhee’s picture

pvhee commented

Thanks Marc, I did a test install on my localhost and found out a couple of issues:

  • When the percentage gets > 100 I get a warning (but I am still allowed to save). |When afterwards I try to remove some percentages it breaks and the same values stay in the database
  • When you have multiple content types enabled, you share the percentages for eg the Title field (which occurs in both types), although I've only set a percent for one of the two. I checked the database and you could perfectly save a percentage for each field in each content type, so I guess there is some bug in the form?
  • In content_complete_menu a new menu item has been added (description 'Allows external applications to communicate with Drupal'). What is the reason for this?
  • We need to provide an upgrade path to add the new field(s) in the database

Thanks for the effort!

marc.groth’s picture

marc.groth commented
Status: Needs review » Needs work

Thanks for those bug reports pvhee... Here are my responses :)

  • The warning message is just an added extra for the users convenience, in order to inform them whether they are under/over (in case they have many different content types each with only a few percentages to each and don't want to add them up manually). I noticed the values weren't changing for me either until I "fixed" the second point, which seems to have fixed this.
  • I took a look at the code and it doesn't actually differentiate between content names for content types (which is silly, because it is highly likely someone will have the same name for multiple types)... When I went to start fixing this it opened up a whole can of worms!! I noticed that "Title" (in particular) wasn't working correctly because it is not a CCK content type. The way the extra code has been written basically expects it to be... Now my question to you is, is this correct? Or should it be that ANY field can have a percentage added to it? If it's the former than it will be a simple case of filtering the results on "Fields" to only show CCK fields... If it's the latter, then the whole code is going to need to be looked at again to make sure it doesn't discriminate... Could you confirm which is correct?
  • I have no idea what that menu item was doing there but I have removed it now, haha thanks for spotting it!
  • That patch should have included the addition of the new field (percentage) in the database. Or do you mean something else?

I appreciate you looking into this and look forward to hearing from you at your earliest convenience (especially the second point). Would really love to get this functionality included into core as I think it's something people could really make use of.

Thanks again and happy easter!! :)

Marc

P.S - I am not attaching a patch of what I've done so far as I feel I need to know more (again, especially on the second point) so that I can fix the code and have it stable before patching it up and re-releasing.

pvhee’s picture

pvhee commented

Hello Marc,

Here some more feedback.

Content Complete works with CCK fields plus the core fields such as Title, Body and File Attachments, which are no CCK fields (in Drupal 7, with fields in core, this will likely change). This difference of course adds an extra layer of complexity, and most of this logic in done in the functions content_complete_get_fields (giving back all fields for a certain type) and content_complete_get_user_fields (giving back all the completed fields for a certain type). You will need to have the same logic for the patch as well I am afraid..

Regarding the upgrade path, I've seen that you included it in the install file but whenever we include this patch in core we will need to have an upgrade path (i.e. to add the field for people that have already installed the module).

Thanks!

marc.groth’s picture

marc.groth commented
StatusFileSize
new content_complete_percentages_6.x-1.x-dev_3.patch17.83 KB

Hey again pvhee!

After some serious hair tearing, I managed to successfully create a patch. The new functionality now includes the ability to give each field a specific percentage regardless of it's name etc. (it's now unique! :)) - this includes non-CCK fields...

As I say, I'm still fairly new to Drupal so could you help me out a little more with the upgrade path? Would I create a separate file for this or would it go in the main .module file? Would it be similar to the edited .install file? Sorry, I have looked on Drupal for any help but am not entirely sure what I'm looking for... Any chance you (or someone else) can point me in the right direction? Thanks in advance :)

I look forward to hearing from you for any comments etc. on the patch... Hopefully we're closer :)

Cheers,

Marc

marc.groth’s picture

marc.groth commented
StatusFileSize
new content_complete_percentages_6.x-1.x-dev_4.patch17.55 KB

Sorry I just noticed that that last patch "_3" had some issues (it was pulling in ALL fields, whether they were selected or not) for some reason. Attached is a patch that deals with that, that is now complete... So please use this latest one and let me know of any problems etc. that you may run into!

Cheers :)

marc.groth’s picture

marc.groth commented
StatusFileSize
new content_complete_percentages_6.x-1.x-dev_5.patch30.58 KB

Hi again,

We have added more functionality, where you can now specify certain fields/percentages for a specified user role... I have attached a patch which includes this. It also now has an upgrade path... So once you have patched this latest version please make sure you run update.php to make sure you get the necessary columns in the database etc.

PLEASE NOTE: This patch is an extension of the previous ones... Therefore the new functionality also includes being able to specify percentages for each field (uniquely) as well as the newly added Roles setting stuff.

I will keep an eye on this page for any comments/suggestions/problems etc...

Thanks :)

marc.groth’s picture

marc.groth commented
Status: Needs work » Needs review

Bumping :)

pvhee’s picture

pvhee commented
StatusFileSize
new Content Complete - contradiction in percentages89.25 KB

Hey Marc, thanks for the patch. I found a little time to review it, and here are my comments (after patching the latest dev + running update.php, which all went fine).

  • The individual percentages I've put for my CCK were not reflected in the block. I put something like 50-33-17, but the next percentage was showing "33", although "50" was expected. I've disabled/enabled the block afterwards but no change. I've attached a screenshot as illustration
  • The administration should be easier and more straightforward in my opinion. The first page of CC should be "Fields" (not "Roles"), the "Percentages" tab could be integrated with the "Fields" tab (with percentages next to the fields), and I don't like so much the nested fields inside the roles on the "Fields" tab. After having enabled something CC stuff, it should also automatically make those fields not collapsed anymore. I am not sure what would be a better interface, but I think we can come up with something that binds "Fields", "Roles" and "Percentages" in one admin page and not 3 interface-unrelated tabs.

I am most likely going to work on #371479: Content Completeness calculation on a per-node basis next to the per-type basis in the upcoming weeks, so expect some changes to happen, which will result in 6.2 of the module. However, I still very much want your contributions inside, but only if we ironed out all the issues.

pvhee’s picture

pvhee commented

Note that version 1.2-beta1 has been released with a lot of new features (views integration is just one of them), and a lot of changes under the hood. Please reroll any patches against this version. Thanks.

pvhee’s picture

pvhee commented
Title: Patch for 6.x-1.x-dev » Percentage per field: Patch for 6.x-1.x-dev

Changing title to better reflect the feature.

marc.groth’s picture

marc.groth commented

Hi pvhee,

Just wanted you to know that I have not forgotten about this. It's just been very busy at work and we haven't really needed this functionality just yet.

It will probably be best if we wait until 6.2 is released as it seems there are going to be quite a few changes (as you noted) and by then we may be in a better situation to try and tackle this problem

Thanks :)

pvhee’s picture

pvhee commented

Hey Marc, good to know! Note that 1.2 has already been released: http://drupal.org/project/content_complete

marc.groth’s picture

marc.groth commented
Status: Needs review » Postponed

Haha sorry, my bad... I saw it but it didn't click that that was 1.2... It's been a long day/week!

Looking forward to working some more on this... I'll update this post as and when I get done. Setting the status to "postponed" until I have another patch for review.

Cheers :)

deggertsen’s picture

deggertsen commented

subscribe