Currently just changes to get the pages/forms working with recent API changes. UI is working correctly AFAIK.

The other item for consideration in moving to 5.x is whether TAC should be made to work alongside the na_arbitrator patch in core. Looking at node.module, I see that (the poorly named) node_access_acquire_grants and node_access_write_grants are called whenever a node is saved, so I think TAC needs to be updated to use the new API in order to work correctly. I'll start looking at how this would work (I suspect it should be pretty simple to port).

On another note, for testing core functionality it seems like TAC is sufficiently complex that it would benefit from some simpletest test cases.

CommentFileSizeAuthor
#2 tac_edrex.patch1.42 KBkeve
tac_5x.patch.txt13.24 KBedrex

Comments

keve’s picture

keve commented

Many thanks for your patch. I will try to look through it.

I think, TAC MUST use the new na_arbitrator API. But recently i had very little time to work with TAC. As i loooked though na_arbitrator, it will not be very complicated to update to use the new API. I just did not have the energy to write the code for it.

I have not try simpletest yet, but thanks for the suggestion.

keve’s picture

keve commented
Status: Needs review » Active
StatusFileSize
new tac_edrex.patch1.42 KB

I commited to HEAD with some minor changes.
(Edrex: See patch. This is a patch against your patch.)

1.The title of Settings page should be 'Taxonomy access settings' instead of 'Taxonomy access status', because later I want to add other settings there. (Also the status will be deleted later, i think, when we integrate na_arbitrator).

2. I changed back some lines in hook_form_alter, of course without the module dependency check. (Module cannot be disabled, until deactivated on settings page.)

I leave this issue active, possibly waiting for other comments + patches for porting to 5.x.

keve’s picture

keve commented
Status: Active » Fixed
Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)
greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

@keve - if this seems like it will work with 5.x (or you want more testers) I suggest the use of a DRUPAL-5 branch and associated release node so that users will see a -dev tarball available, download it, and try it out.

I'm looking forward to using this under 5 and will report any problems I have. Thanks!

karldied’s picture

karldied commented

Installed on test site running v5 rc1, seems to work. Thanks for the update! Agree would be easier to find if marked '5.x' instead of 'HEAD.'

keve’s picture

keve commented

Sorry for my delay. I wanted to look through the code for some optiamization, but i have no time.
I tag it to 5.x.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

Well, it's still a "development" release.

Once you are happy with it (and perhaps we get more feedback from testers) then you can consider a tag and "official" release.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented

And most importantly - thanks for your work on the upgrade and the branch!