I think it will be nice if profile2 keeps the versions for updated profile fields with updater user id like node versions. If it is done it will do the job of D6 content_profile module completely.

CommentFileSizeAuthor
#111 profile2-revisions-1043128-111.patch33.41 KBrickj
#110 1043128-menu-access-interdiff.patch753 bytesdjdevin
#109 1043128-menu-access-interdiff.patch890 bytesdjdevin
#107 profile2-revisions-1043128-107.patch29.68 KBjunaidpv
#104 profile2-revision+user_revision.patch20.97 KBbedstvie
#101 profile2-revision+user_revision.patch19.04 KBbedstvie
#100 profile2-revisions-1043128-100.patch18.12 KBlamp5
#96 Screen Shot 2017-04-04 at 16.23.46.png242.73 KBibuildit
#93 profile2-revisions-1043128-93.patch19.9 KBbocaj
#90 profile2-revisions-1043128-90.patch20.32 KBkylesmith
#73 profile2-revisions-1043128-73.patch21.3 KBmcalabrese
#72 profile2-revisions-1043128-72.patch19.15 KBmcalabrese
#68 profile2-revisions-1043128-68.patch19.01 KBmdeltito
#63 profile2bug.PNG7.26 KBjetwodru
#62 profile2revisions.PNG26.84 KBjetwodru
#62 node revisions.PNG23.95 KBjetwodru
#55 profile2-revisions-1043128-55.patch17.59 KBbenjifisher
#55 interdiff-50-55.txt17.67 KBbenjifisher
#50 profile2-revisions-1043128-50.patch17.59 KBbenjifisher
#50 interdiff-48-50.txt619 bytesbenjifisher
#46 profile2-revisions-1043128-46.patch17.51 KBbenjifisher
#43 profile2-revisions-1043128-43.patch16.5 KBbenjifisher
#41 profile2-revisions-1043128-41.patch16.18 KBbenjifisher
#27 profile2-1043128-revision_support.patch17.1 KBsadashiv
#16 revision support.patch12.26 KBmassud
#13 profile2-revisions-1043128-13.patch11.55 KBrooby
#10 profile2-revisions-1043128-10.patch9.83 KBrooby
#9 profile2-revisions-1043128-9.patch6.59 KBrooby

Comments

joachim’s picture

Versioning should really be implemented at the entity level, not just for profile.

tutumlum’s picture

I don't know much about entity module. And I don't know entity module has native support for field versions or not. So, what does fago think about it as developer of both modules? Is there any plan for that?

fago’s picture

Title: Profile Field Versions » Profile revisions

There is an issue for that in the entity api queue I think.

geerlingguy’s picture

Maybe you'd be interested in helping get this project off the ground: User Revision. Right now it seems to work great with core fields and revisions, but there are some bugs with the module in tandem with Profile2 (notably: #1199528: Notice: Undefined variable: picture in _user_save_revision() (When used with user profile categories)).

k_zoltan’s picture

[#2] I just found this http://drupal.org/node/1129192 it seems to be fairly easy to implement the revisioning for the profile from the database and entity perspective. I don't really know what this involves from the User interface perspective.

fago’s picture

tutumlum’s picture

#5: Support Thanks, k_zoltan. I will try it.

k_zoltan’s picture

Maybe I can help too. Although I have very little time.

rooby’s picture

Status: Active » Needs work
StatusFileSize
new6.59 KB

Here is a quick something I put together to test the patch in #996696: Support revisions in Entity API

It's not a proper solution but just a start and there are probably mistakes due to it being past bed time but it did I did create a profile and update it and I got revisioning, yay :)

Creating a new revision on every save is currently hard coded in though.
It still needs work and it needs me to have another look over it with fresh eyes tomorrow.

rooby’s picture

StatusFileSize
new9.83 KB

Here is a better patch.

It has schema improvements, a better update function, and has proper revision functionality, ie:
* You can set a default per profile type for whether or not to create revisions
* You can override the default revision setting on the profile edit form and also add a log message (if you have the required permissions).

It still doesn't have any 'view revisions' functionality, like a permission to view revisions or a page to view revisions.
It also has no tests yet and I haven't had a chance to do much testing.

rooby’s picture

Status: Needs work » Needs review

Actually, any feedback would be good and it would be nice to see test results, so changing status.

Status: Needs review » Needs work

The last submitted patch, profile2-revisions-1043128-10.patch, failed testing.

rooby’s picture

Status: Needs work » Needs review
StatusFileSize
new11.55 KB

Here's a new version that fixes a few obvious problems with the last one.

It's still a rush/hack job but it is better than the last one.

Status: Needs review » Needs work

The last submitted patch, profile2-revisions-1043128-13.patch, failed testing.

rooby’s picture

Marked #1150568: Support revisions as a duplicate of this issue.

massud’s picture

StatusFileSize
new12.26 KB

The attached patch is a complete reimplementation of the patch provided in #13. This patch is against profile2 7.x-1.2
Notice that this is based on #996696: Support revisions in Entity API provided in #92 with the fix provided in #101.

rooby’s picture

Great, thank you for this.

I will test sometime this week.

yesct’s picture

rooby, what is the strategy to implement revisions with the latest patch? what should I look for when testing it?

matt_paz’s picture

Just tested the patch from #16. I looked at the db ... it appears that revisions are being record. It doesn't look like there is a way to leverage revisions in views or to view or diff revisions from a menu item yet, but this is great progress.

Thanks!

matt_paz’s picture

oops ... i take that back. Just noticed that when I edited a profile other than my own, it wiped out my profile and replaced it with the contents of the one I was editing. i haven't investigated the root cause ... could be entityapi or could be profile2 ... just wanted to relay that.

Anonymous’s picture

@matt_paz I think the profile ownership problem in #16 stems from the following code:

// Handle revisions.
   if (!empty($form_state['values']['profile_' . $type]['revision_information']['revision'])) {
     global $user;
     $profile->uid = $user->uid;
     $profile->timestamp = REQUEST_TIME;
     $profile->log = empty($form_state['values']['profile_' . $type]['revision_information']['log']) ? 
         '' : $form_state['values']['profile_' . $type]['revision_information']['log'];
     $profile->is_new_revision = TRUE;
   }

Should probably check the $profile->authorid as in #13
$profile->authorid = isset($profile->authorid) ? $profile->authorid : $user->uid;

rlmumford’s picture

Entity API now supports revisions, so this patch should be refactored to make use of that work. #996696: Support revisions in Entity API

ravisagar’s picture

Hey,

I am using profile2 now I want the ability for the admin to be able to view the revisions for multiple profiles associated with the user. Is there any interface for that in the profile2?

The facility to view revisions for Nodes is something I am seeking in profile2.

-Ravi

rooby’s picture

That is what this issue is about but it isn't finished yet.

The patch in #16 was created before the entity revisions patch was finished so it probably needs a few tweaks.

It also needs lots of testing.

ravisagar’s picture

Hi rooby,

I just applied the #16 patch. I found one bug while testing it.

When I select "Create a new revision" and update the field then I get the following message.

Notice: Undefined index: revisions in profile2_attach_form() (line 437 of /PATH/sites/all/modules/profile2/profile2.module).

and all the values in the fields are cleared!!

ravisagar’s picture

One more thing is that if as Admin I update different profile then both the profile show my data (admin's)

sadashiv’s picture

Status: Needs work » Needs review
StatusFileSize
new17.1 KB

Patch at #16 worked for me, only problem is that can't view previous revisions. I tried to use patch at #16 and add revision viewing support I am attaching patch for this.
Please run update.php after applying the patch else error at #25 occurs.
May be needs more improvement

Thanks,
Sadashiv.

Status: Needs review » Needs work

The last submitted patch, profile2-1043128-revision_support.patch, failed testing.

sadashiv’s picture

The patch at #27 fails because of update.php not ran before testing. You may try running update.php and then proceed.

Hth,
Sadashiv

sadashiv’s picture

Status: Needs work » Needs review
ravisagar’s picture

I will test it. Thanks for that info.

andrewko’s picture

Is the patch in #27 using the new Entity API revision support or does it still need to be refactored?

rooby’s picture

It is using entity api revision support but off the top of my head I think it was done before the entity api stuff was fully finalised, so it definitely needs to be checked over and might need some changes.

geek-merlin’s picture

any movement on this?

rooby’s picture

I will soon be restarting work on a project that is using an old version of this and will need to be updated, but I hope someone beats me to it.

geek-merlin’s picture

@rooby: feel free to pm me for help (test, review).
(you really want help beating you? my physical location is too far away ;-))

benjifisher’s picture

I may be able to do some work on this.

Looking over the comments and the patch from #27, it seems that the problem reported in #20 and #26 will still be there: see the fix suggested in #21. As an admin user, try editing another user's profile and see what happens.

benjifisher’s picture

Issue tags: +some testing

The patch from #27 is not working very well for me.

I am not sure if the suggestion in #21 is on target. The patch in #13 adds a column 'authorid' to the profile_revision table, whereas the patch in #27 adds a column 'uid'. So replacing "authorid" with "uid" is not necessarily an error.

I applied the patch from #27 and ran "drush updatedb". I now have a profile_revision table in my DB; it and the profile table each has a column referencing the other's primary key.

I created a new profile type. I added two fields (text and long text) and gave them default values. When I edit and save (user/28646/edit/test_profile_type), the default values still show up. This should not be hard to fix, and it might explain the comments in #20 and #26.

When I edit and save, nothing gets added to the profile_revision table. Instead, a new row is added to the profile table (with a new pid and with vid = NULL).

benjifisher’s picture

function benjifisher_remove_foot_from_mouth() {
  exec('drush update entity');
}

In other words, I get better results if I use the latest version of Entity API. (Installed version was 7.x-1.0-rc3.)

I do still see the problem reported in #20 and #26. Looking at the database, I see that the uid columns in both the profile table and the profile_revision table get updated when an admin user edits another user's profile.

As long as I stick to editing my own profile, things work as expected.

geek-merlin’s picture

Status: Needs review » Needs work

the suggestion in #21 should read:
in profile2_form_submit_build_profile() replace

$profile->uid = $user->uid;

with

$profile->uid = isset($profile->uid) ? $profile->uid : $user->uid; 

and this should fix the admin-edit problem reported in #26

benjifisher’s picture

StatusFileSize
new16.18 KB

I started working on this before reading the comment in #40. Based on the description attached to the uid/authorid column in the profile_revision table, I think the intent was to keep track of the user who created the revision, so I went with the fix in #21.

Looking through the code (to be specific, DrupalDefaultEntityController::buildQuery() in entity.inc) it looks as though columns (or should I call them fields?) in an entity's revision table will override like-named columns in the base table when the entity is loaded. Maybe there is a way to change this behavior, but changing uid to authorid seems the simplest approach.

The attached patch is a work in progress, so I will leave the status as NW. (But feel free to test!) I think it is an improvement on #27. In initial testing, it seems to solve the problem of an admin user stealing the profile when (s)he edits it.

Some oddities (should not be too hard to fix):

  1. When I run the update function and when I create a new profile type, my admin role does not get associated permissions automatically. This may be something funky on my site.
  2. When I create my first profile and (as an admin) un-check the box to create a new revision, I get some sort of PHP error. If a user does not yet have a profile of the new type, I do not think there should be an option not to create a revision.

I copied the name of the field from @rooby's patch in #13 and I also took the Doxygen comment on the update function from there.

Note that a lot of the code is similar to the entity_test.module code that comes with the Entity API.

One difference between the patches in #13 and #27 is the properties (not null, unsigned) attached to some of the database columns. Any advice on this?

benjifisher’s picture

I forgot to mention two things.

A user who does not have permission to view revisions still gets the "View Revisions" link on his profile page. Maybe even for profile types that do not allow revisions. Should be easy to fix. I do not think this is my fault, but maybe it is.

While I am working in the install file, it is tempting to add a call to field_attach_delete_bundle() in hook_uninstall() as required in this change notice. It is really an unrelated problem, so I opened a new issue for it: #2051371: Call field_attach_delete_bundle() in hook_uninstall().

I will try to have an improved patch ready later today.

benjifisher’s picture

Status: Needs work » Needs review
StatusFileSize
new16.5 KB

OK, I am ready for testing. This time, I remembered to generate a patch relative to the profile2/ directory instead of my site root.

I would like to know whether others have the same permission problem I mentioned in #41.

I think I fixed the other problem that I mentioned in #41.

As I suggested in #42, I removed the "View Revisions" link from the profile page if the user does not have permission to view revisions.

When I wrote #42, I thought that "Create new revision" checkbox on the profile-type-edit page indicated whether the profile type supports revisions. I see now that all types support revisions, and this checkbox just indicates the default value for the revision checkbox on the profile-edit form. (If a user does not have the "administer profiles" permission, then the default is the only option.)

If you want finer permissions, such as "view own/any profile revisions" or "view $type profile revisions", or if you think that these permissions should be integrated with hook_profile2_access(), please file a follow-up issue or submit a patch on this one.

Status: Needs review » Needs work

The last submitted patch, profile2-revisions-1043128-43.patch, failed testing.

rooby’s picture

Thanks for your work!

I haven't properly tested or reviewed yet but just checked that last error.

The error is because the tests are saving a profile that doesn't have a log message set at all.

I would say that profile2_save() should be checking the log value and setting to '' when there is no log, like node_save() does. Also, see the profile2_save() part of the patch in #13.

There could easily be code in other contrib modules and people's custom modules that will be saving profiles without a log.
Becuase it is a text field in the db we can't give it a default so we have to always make sure it has a default value.

benjifisher’s picture

StatusFileSize
new17.51 KB

A few more changes:

  1. Set the 'revisions' key in hook_install(). This is already done in hook_update_N(): see the comment in #27 about running update.php. In automated testing, we install from scratch, so this change is needed to pass.
  2. Set a default $values['log'] = 1; in the Profile constructor. I think this would also solve the second problem I listed in #41.
  3. I commented out some lines that appear in the patches from #13 and #27 (and all of mine and probably others).
    // The following lines were part of the proposed patch from #13 and #27 but I do not see that
    // this is ever used, and it is odd to set $form['pid'] and $form['vid']
    // inside the foreach ($form_state['profiles']) loop.
    //
    // foreach (array('pid', 'vid') as $key) {
    //   $form[$key] = array(
    //     '#type' => 'value',
    //     '#value' => isset($profile->$key) ? $profile->$key : NULL,
    //   );
    // }

Oops, I just noticed #45. Yes, I think that setting a default in profile2_save() would also work. Since I already did it in the Profile constructor, I am inclined to leave it that way, but if you think it makes a difference I am happy to learn.

benjifisher’s picture

Status: Needs work » Needs review

Have at it, testbot!

benjifisher’s picture

@rooby:

I just looked at the user_save() code from your patch in #13. It seems to have been lost by the time we get to #27. In the unpatched module, I see some such code in the Profile constructor; as Drupal becomes more OO, I think the idea is to keep things like profile2_save() as pure wrapper functions, with the logic inside the class. Any arguments on the other side?

We also seem to have lost the 'ip' column in the {profile_revision} table. It should be easy enough to put it back if you think it is useful.

Please also note my last question in #41. And the problem I have with new permissions not being assigned to the admin role.

rooby’s picture

Thanks, putting the defaults in Profile::save() is good I think.

As for the ip column, I'm not sure where that came from. I probably saw it in some other revision implementation.
It wouldn't be a bad thing to have but I'm not tied to it. Other revisions, like node revisions, don't capture that.

I'll have to test the permissions issue you were seeing to see if I can reproduce it.

I had a quick look at the schema and I think your properties are good.

A couple of other things though:

The profile table should get the unique key for vid in the schema.
Currently it only gets it if you are updating via update 7103.

+++ b/profile2.install
@@ -144,6 +151,93 @@ function profile2_schema() {
+      'version_uid' => array(
+        'table' => 'users',
+        'columns' => array('authorid' => 'authorid'),
+      ),

The columns here should be:
'columns' => array('authorid' => 'uid'),

as the target column is users.uid

benjifisher’s picture

StatusFileSize
new619 bytes
new17.59 KB

@rooby:

Thanks for the advice. I have attached an updated patch and an interdiff. It still passes automated tests locally.

What about the 'label' column? It looks as though all it does is copy the label from the {profile_type} table. If I edit the profile type, then this does not get updated in {profile} nor {profile_revision}, so storing the label rather than the {profile_type} 'id' seems really odd. The 'type' is already stored. Is there some other way to modify 'label' in the {profile} or {profile_revision} table?

Anyway, do we really need to keep revisions of 'label' in the {profile_revision} table?

rooby’s picture

The label is there because label is also a field on the profile.

When a new profile is created it is assigned a label from its profile type.

I tracked down the issue where that was added (#1338968: Use tokens in profile type label) and it seems it is there so that people can save their own labels into it using rules or other custom methods.

So I guess it comes down to whether or not that needs to be revisioned.
For the sake of tracking all changes I would probably say it should be.

Keen to hear other opinions on this though.

benjifisher’s picture

Status: Needs review » Needs work

Obviously, this patch needs some automated tests. I copied the tests from the node module, then modified them. This exposed a problem: if I

  1. create a revision and dutifully include a log message
  2. edit the profile without creating a new revision,

then the log message is lost.

I looked into it, and I think the problem is that the profile object returned by profile2_load() (which calls profile2_load_multiple(), core entity_load(), EntityAPIController::load()) does not include the log message. All the other columns from the profile_revision table are there, along with attached fields, but log seems to get special treatment for some reason.

benjifisher’s picture

@rooby:

Continuing the discussion from #51, I agree that labels should probably be attached to revisions, and I am also willing to get opinions from those who actually use this feature.

Other than the pid and vid, the profile and profile_revision tables should not share any keys. Values in the revision table will override values in the base table, thanks to these lines from DrupalDefaultEntityController::buildQuery():

      // Add all fields from the {entity_revision} table.
      $entity_revision_fields = drupal_map_assoc($this->entityInfo['schema_fields_sql']['revision table']);
      // The id field is provided by entity, so remove it.
      unset($entity_revision_fields[$this->idKey]);

      // Remove all fields from the base table that are also fields by the same
      // name in the revision table.
      $entity_field_keys = array_flip($entity_fields);
      foreach ($entity_revision_fields as $key => $name) {
        if (isset($entity_field_keys[$name])) {
          unset($entity_fields[$entity_field_keys[$name]]);
        }
      }

So profile2_load() will never query profile.label if profile_revision.label exists.

Assuming that we decide to attach labels to profile revisions, I think we should remove the label column from the profile table. (In hook_update_N(), copy over the existing values first.) There is some danger that this will break other modules, if they use direct SQL queries that assume a label field on the profile table.

benjifisher’s picture

I tracked down the problem with the disappearing log message (see #52), and it seems to be my own fault. See the discussion of default value for the log message in #45, #46: maybe @rooby was right!

I think the problem comes from this line in EntityAPIController::query():

      $result->setFetchMode(PDO::FETCH_CLASS, $this->entityInfo['entity class'], array(array(), $this->entityType));

When the query is executed, it seems that the default Profile constructor (no arguments) is called before the query results are fed into it. Not at all what I would expect, but adding a debug line to the constructor shows that the $values parameter is empty.

benjifisher’s picture

Status: Needs work » Needs review
StatusFileSize
new17.67 KB
new17.59 KB

@rooby, @axel.rutz:

How about some testing? I think the attached patch is close to being ready.

Here are the main differences since the previous patch (#50):

  1. Remove the label field from the profile table. (See #53 above.) This may break modules that use custom SQL queries instead of calling profile2_load().
  2. Add automated tests (See #52.)
  3. Do not set a default value for the log message in the Profile constructor. Instead, do it in Profile::save(). (See #45, #46, #54.)
  4. Rewrite profile2_revision_overview() (page callback for profile/%profile2/revisions) to use profile2_load() instead of a complex, custom SQL query.
  5. Add profile2_view() (page callback for profile/%profile2/revisions/%/view).
  6. Make status messages ("Revision deleted...") more detailed.
  7. Rearrange profile2_update_7103().
  8. Several formatting changes (improvements, I hope).
rooby’s picture

Like you say, the only argument I could see for the label being in both tables would be if you ever were directly querying the table and want the label, in the same way you directly query the node table for the title or something.

From what I can gather the main use of the label relates to rules, in which case the full entity will be loaded.
I think it's probably fine to keep it in revision only.

Fago can probably comment on this once it is time for his review.

I think profile::save() is a good place for forcing defaults.

profile2_save() is definitely not good as it doesn't get fired in all cases of saving a profile, and there are a few ways things can get messed up when setting it on creation (it could be unset at some point before saving and cause errors too).

I will try to make some time to review over the next few days.

rooby’s picture

I've had a bit of a test of this, but I haven't rolled it into my real deal site yet.

The update function ran without error.

The revisioning functionality seems to work fine. I've tested with different combinations of updated and non-updated fields, with and without revision log, with and without saving revisions, with users who have admin rights and with users that don't.

Viewing an old revision worked.

Reverting a revision worked.

Things that I feel have room for improvement (others may disagree):
* The revisions admin screens seem a bit detached from the user account area (different url scheme and the active menu trail is no longer the user section). It's possible the profile2_page module would also want them in a different place, however that is probably something for profile2_page to deal with later as a follow up if it is something people want.
* The revision listing for nodes shows a date and author of each revision, which I think would also be useful here.
* When you revert a revision the message is "Copy of the revision.", which doesn't mean much. The node one is "Copy of the revision from Sat, 12/01/2012 - 19:04.", which is more meaningful. This ties in with the above point.
* When inserting revisions for existing profiles during the update, should it use the profile changed date instead of the current time?

I am still yet to do a proper code review.

jetwodru’s picture

Hi,
I'm confused, look like Entity API had supported Revision at https://drupal.org/node/996696 , should I still need to update this patch to profile2 ? Could Profile2 support revision now after using the latest version of Entity API ?

I just need a history of changes to prevent the loss of original data upon overwriting by users. Kindly enlighten, thanks

benjifisher’s picture

@jetwong98:

I think that the patch in #55 will do what you want. It uses the revisioning support in Entity API; without the patch to Profile2, I do not think you will get anything out of the updates to Entity API.

jetwodru’s picture

Hi,
Thanks but after patching and running the database update, I encountered the following errors

Warning: array_keys() expects parameter 1 to be array, null given in drupal_schema_fields_sql() (line 7033 of /public_html/includes/common.inc).

Warning: array_keys() expects parameter 1 to be array, null given in drupal_schema_fields_sql() (line 7033 of /public_html/includes/common.inc).

Warning: array_keys() expects parameter 1 to be array, null given in drupal_schema_fields_sql() (line 7033 of /public_html/includes/common.inc).

Now I can enable create revision but I found the interface very odd. View Revision wording appears beneath the field which had been modified. It doesn't look like the node revision. I'd prefer to have a Unified Revision Interface for All Entities (like node) for the ease of maintenance together with Diff module. Just my 2 cents, thanks

jetwodru’s picture

Agree with #57,

1.) The revision URL is detached from my domain.com/personal-business_profile/158 to domain.com/profile/5/revisions . When clicking the revision, the trace is lost.

2.) No date and time on each revision made in listing, very difficult to check.

3.) No integration with Diff module, not like Node revision.

Thanks

jetwodru’s picture

Issue summary: View changes
StatusFileSize
new23.95 KB
new26.84 KB

Hi,
This is a comparison of current profile2 and node revision interface. I'd prefer Profile2 Revision to be designed the way like Node Revision as follows. Thanks

Profile 2 Revision Interface (not intuitive & quite confusing)
profile2revisions


Node Revision Interface (clean & intuitive)
node revisions

jetwodru’s picture

StatusFileSize
new7.26 KB

Hi,
A bug found, no revision but the View Revision link still apears and then leading to Page Not Found when clicked.

profile2bug

igorik’s picture

Hi

I am a bit confused.
While profile (profile 2 module) is a entity, why is not default entity revision enough?

thanks
Igor

kt2ssh’s picture

image field is not working yet ?

record is inserted , but when loading revision

$profile = profile2_load(249471, 249510));
$profile['field_image']['und'][0]['fid'] = 0 (NULL)

Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 173 of /home/scmp/youngpost/htdocs/includes/entity.inc).

spleshka’s picture

Guys, any progress in this issue? I would love to commit final patch to enable revisions for profile :)

spleshka’s picture

Status: Needs review » Needs work

I think it makes sense to change issue status to "Needs work", because we don't have a proper solution for a review.

mdeltito’s picture

StatusFileSize
new19.01 KB

reroll #55 against HEAD

The last submitted patch, 55: profile2-revisions-1043128-55.patch, failed testing.

gladiatorhl2’s picture

Does it work or not? It is not clear but it is seen that there are some errors.

mcalabrese’s picture

StatusFileSize
new19.15 KB

in regards to #60 I had the same errors occur.
This is due to inconsistencies between the profile table and the profile_revision table.
For some reason NULL values are allowed in profile.uid but not in profile_revision.authorid.
I'm not sure why you would want NULL values in profile.uid.
I made a simple fix by not allowing profiles with profile.uid to have an entry in the profile_revision table.

mcalabrese’s picture

StatusFileSize
new21.3 KB

adding views support of profile_revision fields

jetwodru’s picture

hi mcalabrese,
is profile2-revisions-1043128-73.patch a cumulative update ? tks

rooby’s picture

Based on patch file size I would say yes. Try it and see.

spleshka’s picture

Status: Needs work » Needs review

I think this patch definately needs review from the community.

The last submitted patch, 68: profile2-revisions-1043128-68.patch, failed testing.

The last submitted patch, 72: profile2-revisions-1043128-72.patch, failed testing.

dubs’s picture

This is a great patch - it would probably benefit from checking the vid when a user loads their own profile so they can see the most current revision - would that be possible? In our simple use case we have the following function which augments the profile2_load_by_user function: -

<?php
/**
 * Check which vid to load depending on profile being viewed and permissions
 */
function profile2_get_revision_vid($uid, $pid) {
  global $user;
  // quit of the current user is viewing another profile
  if ($user->uid != $uid) return;
  // check for the latest VID
  $profile = new stdClass();
  $profile->pid = $pid;
  $revisions = profile2_revision_list($profile);
  if (isset($revisions)) return reset($revisions)->vid;
}
?>

Our use case is with profile2_moderation so people can preview their own changes.

rooby’s picture

But shouldn't the latest revision always load by default?
Unless you specifically load a different revision.

That's how revisions generally work everywhere else.

dubs’s picture

@rooby - yes, your right but in our case this is used in conjunction with profiles2_moderation module which doesn't use the current revision until it's been approved.

igorik’s picture

can we add it into the Profile 2 module?

igorik’s picture

Status: Needs review » Reviewed & tested by the community

patch #73 is the final? Is ithis what is used for Profile 2 moderation module?

spleshka’s picture

@igorik, have you tested this patch youself?

paulwdru’s picture

Revision is very critical providing some sort of Audit Trail, dunno why it was missing in Entity earlier.

dromansab’s picture

I cannot apply the patch on the last dev version.

Sorry... It seems to work, but with some warnings:

Warning: Illegal string offset 'und' in profile2_moderation_get_revisions_diff() (line 131 of (...)/sites/all/modules/profile2_moderation/profile2_moderation.pages.inc).
Warning: Illegal string offset 'value' in profile2_moderation_get_revisions_diff() (line 131 of (...)/sites/all/modules/profile2_moderation/profile2_moderation.pages.inc).
Warning: Illegal string offset 'und' in profile2_moderation_get_revisions_diff() (line 137 of (...)/sites/all/modules/profile2_moderation/profile2_moderation.pages.inc).
Warning: Illegal string offset 'value' in profile2_moderation_get_revisions_diff() (line 137 of (...)/sites/all/modules/profile2_moderation/profile2_moderation.pages.inc).

paulwdru’s picture

Hi,
Is this added to the latest Dev version ? I saw a recent update in the Dev

igorik’s picture

would be great to have it in dev release!

kylesmith’s picture

StatusFileSize
new20.32 KB

I'm using the release Profile2 module (version 7.x-1.3) and have recreated patch #73 so the profile revisions patch can be applied to it.

The only other change I made was to set the weight of the 'View Revisions' link in profile2_entity_view() to 999 instead of 10 (line 477 in my patch), so the link should always appear below the other profile fields.

The patch is working great so far.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 90: profile2-revisions-1043128-90.patch, failed testing.

socialnicheguru’s picture

Patch 73 I am getting a WSOD:
PHP Parse error: syntax error, unexpected 'public' (T_PUBLIC) in modules/all/profile2/profile2.module on line 1147

The line is:
profile2-revisions-1043128-73.patch:+ public function revisionDelete($revision_id) {

bocaj’s picture

Status: Needs work » Needs review
StatusFileSize
new19.9 KB

I needed this for a project and tried to apply #73, but it wouldn't apply cleanly. I manually applied it and rerolled the patch. During testing I also noticed an issue with profiles that had the "Create new revision" checkbox unchecked in the profile edit form. An error was thrown on the user's profile page. I added checks for that option. Otherwise no changes from #73. In my testing it appears to function well.

shi99’s picture

#93 worked for me.
I can now create revisions for profile 2.

dagomar’s picture

The patch in #93 didn't apply cleanly for me, the info file has a change (no newline at end of file) which caused it to fail.

The rest did apply and although the tables and functionality is seemingly available, no new revisions are being saved. I'm looking into this.

ibuildit’s picture

StatusFileSize
new242.73 KB

#93 does not apply cleanly to me either. See image.

It does no longer crash from not finding the database table, but revisions moderation does not work for me either. Revisions are being created how ever.

Getting there! Great work so far!

ibuildit’s picture

I figured it out. I used the #55 patch for now.

Then under each profile config page there are two checkboxes to tick (I'm using module profile2_moderation), after that it worked great for me.

pankajsachdeva’s picture

After applied this patch, we have new checkbox on Profile2 module's Profile page like 'Moderate revision' etc. These options are unchecked after every Clear Cache operation.

Can someone help to resolve this issue?

mitjasvab’s picture

@ibuildit you have to apply the patch do the development version 7.x-1.x-dev

lamp5’s picture

StatusFileSize
new18.12 KB

Hi @all. Reroll #93 to the latest dev.

bedstvie’s picture

StatusFileSize
new19.04 KB

Added integration with module User Revision
and fixed problem with updating revision log without creating new revision.

Status: Needs review » Needs work

The last submitted patch, 101: profile2-revision+user_revision.patch, failed testing. View results

bedstvie’s picture

bedstvie’s picture

StatusFileSize
new20.97 KB
rickj’s picture

As current maintainer of this module, I think incorporating this functionality into the build is too high-risk. After quite a long period of relying on dev builds, I've managed to clear several long-standing issues and achieve much better stability.

I suspect it's also a minority requirement, so could it be built as a separate module, like User Revisions? Or maybe it should be an extension to User Revisions rather than Profile2, as people who want profile revisions probably also want user revisions. Perhaps one of the previous contributors would like to take that on?

djdevin’s picture

I don't think it's a good idea to have a [modulename]_revisions for every project that needs revisioning. Making a schema or other change to one project would then require coordination with the other project and the releases would always have to match.

I would like to see it implemented here. With test coverage I don't see why it would be high risk.

junaidpv’s picture

Status: Needs work » Needs review
StatusFileSize
new29.68 KB

Patch from comment#90 had views integration which appears to be missing from later patches.

We are using patch from comment#90 with several other enhancement for past two years on a big Drupal installation. So, decided to re-roll and prepare new patch with following enhancements:

  • Add pid field in views integration. It will help to create relation ship in view to show history of profile.
  • Provide admin configuration to configure whether user is allowed to decide to create revision or not.
  • Show revision in sub-tabs if tab feature is enabled for profiles.
  • Provide pages to view, revert, delete individual revisions.
  • Provide more finer permissions to view, revert, delete individual revisions.

Here is the enhanced patch.

djdevin’s picture

Edit: nevermind, moved to a new issue. Wasn't related to the patch.

djdevin’s picture

StatusFileSize
new890 bytes

A patch on #107 - if a profile has no revisions, the link still shows but returns access denied.

This checks menu access beforehand.

djdevin’s picture

StatusFileSize
new753 bytes

Fixed a notice.

rickj’s picture

StatusFileSize
new33.41 KB

I've been spending some time looking at this patch, and I've tidied several things up. The underlying functionality is no problem, but I found the UI was rather inconsistent and a bit buggy.

In particular I didn't like the inline "view revisions" links, as these are completely different from the way revisions are presented for nodes, as well as in the User Revision module. I've therefore re-worked it so as to use a conventional Revisions tab.

Where profiles are not shown as a separate page, each profile's revision list appears in its own sub-tab, in the same manner as editing, or viewing in tabs. The default tab is "Account", which shows the User Account revisions if User Revision is installed, but if it's not, or there are no Account revisions, the tab must still be present as a place-holder. In that case the Account tab just displays the text "No User Account revisions available". If there are no sub-tabs with content at all, then the main Revisions tab is not displayed.

If you want to use the User Revision module as well, it needs a patch which I've posted in #3102858: Compatibility with Profile2 revisions. Without it there are some UI conflicts.

Please give it a try and let me know your results.

rickj’s picture

Status: Needs review » Closed (outdated)
Related issues: +#3118395: New 7.x-2.0-beta1 release

Profile revisions and diffs are now fully incorporated into release 7.x-2.x. See #3118395: New 7.x-2.0-beta1 release.

Closing this issue as outdated.