Drupal 7 port

There's a plan - I'm trying to get in touch with the person who had volunteered to do the conversion, but haven't had any luck recently.

And yes, of course it needs to be converted to fieldapi for D7.

Furthering up on that, I'd like to nail down the last few problems with link 6.x-2.8 and release 6.x-2.9 properly, and then re-branch the code and start working on changes from there.

No, I don't have an ETA for 2.9. Sadly, there's one or two folks over in #664990: No Rel-Attribute and #626932: Unsupported Operand Types - Bulk URL Aliases who have problems that stem from an un-sufficiently tested bit of code that got rushed into 2.8 due to a security bulletin. That code had been added without simpletests, and it seems to have left behind some odd data which I haven't been able to reproduce so I can work around it.

Also, there's #695272: Make Link module compatible with both, views 2 and views 3 - you *do* want link module to work with views, right?

I also need to figure out what's going on with #628902: warning: preg_match() expects parameter 2 to be string - that may well be related to the first two bugs, but I don't know yet.

Those look like the major items I need to clear off my plate before I can push 2.9 out the door. One way you can help is to review and try out the patch in #695272: Make Link module compatible with both, views 2 and views 3 and let me know if it seems to work fine to you. Even better would be if you were to take some time and make some simpletests that test that the views code is working fine.

Alternately, take a look at some of these other bugs, download the 2.x dev code, and see if you can figure out a way to reliably replicate the errors they're getting.

Thanks.

Wow, I don't get why both link and email module is part of d7 core. They are both very frequently used.

Subscribing

With last week's release of Link 6.x-2.9 and the beta release of Drupal 7 right around the corner, I just wanted to check back in to see if any additional planning has gone into the D7 port.

I see that there is a 7.x-1.x-dev branch of the module, but it looks like it hasn't been updated since last November. Now that Link 6.x-2.9 has been released (including all of its bug fixes), is now the time to get some momentum on making 6.x-2.9 the starting point for the Drupal 7 port?

Thoughts?

Thanks,
Ben

Yes, having pushed 2.9 out the door, I now feel that things are clear enough to start working on the update to Drupal 7. Right now I'm busy working on setting up for my presentation at our local drupalcamp this weekend, so I don't have the time to get into this yet. I'm always welcome to accept patches/help.

The 7.x-1.x-dev branch exists, yes, but it's basically a copy of the old 6.x-2.x-dev branch.

Just a note for anyone else who stumbles in here having grabbed and installed the D7 branch as it stands today -

You can't disable the module using admin/modules/list or drush dis. You will need to do update system set status=0 where name='link'; via MySQL instead.

Of course, if you're trying out D7 you're ready for that.

Here's a patch off the D7 branch which lets you get as far as attaching Link fields to content types. If you actually try to edit the field instance, though, there's some major problems, and the field doesn't show up when you actually try to edit or create content of that type. But baby steps. In both cases, the problem seems to be in the theming layer, so if anyone out there has in-depth knowledge about the various theming layer changes between D6 and D7, please take a peek if you can.

I'll try to put in some more work on this tomorrow, if not tonight.

Okay, field settings work now. The problem seemed to be with the code that styles the "Static Title:" radio button so that the title field is next to the radio button label. After wasting some time trying to get this to work, I gave up and now just have the title field separate. Not as nice, but it works now.

More tomorrow, or maybe later tonight.

Field now shows up on node editing form, and appears to correctly stores values, though the validation function throws errors and the link doesn't properly display on node view.

Settings form validation and various other clean-up. Still not displaying output, though. Is anyone actually following along on this? I'm gonna keep doing it anyway, but out of curiosity…

Garrett, I am following along with this, indeed, and I'm quite happy that you're helping out. Unfortunately at the moment I'm busy between two projects, so I haven't had a chance to look at your code yet. I'm hoping I can find some time this weekend, though.

Again - thanks for the help!
John

Garrett,

Yes, I'm eagerly following along and plan to help with testing. I've been following your comments and was just waiting for your port to get somewhat stable... then I planned on doing some active testing to uncover additional bugs and issues. If you'd like me to begin sooner rather than later, let me know.

Cheers,
Ben

Garrett,

Good work so far on the port!

a few notes:

1) I think renaming the field type from 'link' to 'link_field' in link_field_info() might make the upgrade process more difficult.

2) the field isn't being shown on node view because in link_field_info() you have

      'default_widget' => 'text_textfield',
      'default_formatter' => 'text_default',

so text.module is actually taking over. Attached is an updated patch fixing that part of the module so the link is shown, in case that was blocking you.

3) Maybe we should be starting with the code in CVS for D6? I feel like the D7 code is already pretty old?

1) I think renaming the field type from 'link' to 'link_field' in link_field_info() might make the upgrade process more difficult.

I did this because the theming function for this field was called theme_link(). Unfortunately, this function is now part of core in D7. I figured de-simplifying the field's name would help avoid similar collisions.

3) Maybe we should be starting with the code in CVS for D6? I feel like the D7 code is already pretty old?

I was afraid of that. When I get back to work on this (might be a couple days) I'll look at building patches against the D6 branch instead of the D7 one.

Ben, you say you can help with testing… Do you know how to write Drupal 7 tests? The module should have some at some point.

Untested patch against the D6 branch head. WHY DOES A BUNCH OF CODE IN THE D6 BRANCH HAVE TABS THREE SPACES WIDE WHY WHY WHY

Okay, errors in previous patch are gone. Now roughly to where we were before; data is apparently stored correctly, but no link is being displayed on node view. slip's comment reminded me how anal D7 is about every field having its own formatters, so I've headed in that direction, but it's not working yet.

The Link project page has the D7CX pledge on it. Frankly, I don't think a full D7 release for this module on D7 launch day is likely to happen, particularly if this n00b is the only one working on it.

Don't worry so much about it! D7 hasn't even gotten into a Beta release yet. There's time.

And we'll get it working. I need to clear a project out so I have free time again, and then I'll be helping more. The sad thing about being a freelancer is that when you're looking for work, sometimes you find too much.

Tonight's tweaks. At some point, I seem to have broken the data actually saving itself in the database.

D'oh.

So, Garrett, I'm really enjoying how things are going so far - I haven't had a chance to look at what you've done due to having said 'Yes' to too many projects, but I'm hoping to look soon.

Thing is, for this sort of work, having you post a series of patches seems a bit cruddy and slow. I happen to have a git account, and I'm thinking it might be cool to move the 7 stuff over to git, let us work off of that, and then move the changes back into cvs when we've got something working. What do you think?

Also, on the off chance you're near it, I'll be at DrupalCamp LA this year.

If you think it will help development, sure, we can go the git route. It may be easier to just grant me CVS access to the repository here, though.

Haven't been able to do any work on it recently… I started my new job, and that plus lingering commitments at my old one are keeping me busy. Hope to restart soon, though.

Hmm... I think I might give it a try.

And I understand about how a new job can fark up your schedule.

I have some paid time to work on helping port this module to D7. Could we possibly get the latest patch committed to CVS to help get things rolling?

I have a basic Drupal 7 alpha6 install, w/the following added modules: backup_migrate, imce, migrate, mollom, nodesinblock, options_element, pathauto, permission_select, superfish, taxonomy_manager, token, views, views_bulk_operations, xmlsitemap also added. I added the link module for D7, but when I go to manage fields on a content type to add a link field, I get the following (and no link field loaded in the options):

* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 574 of /.../sites/all/modules/link/link.module).
* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 579 of /.../sites/all/modules/link/link.module).
* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 584 of /.../sites/all/modules/link/link.module).
* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 589 of /.../sites/all/modules/link/link.module).
* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 594 of /.../sites/all/modules/link/link.module).
* Notice: Use of undefined constant CONTENT_HANDLE_CORE - assumed 'CONTENT_HANDLE_CORE' in link_field_formatter_info() (line 599 of /.../sites/all/modules/link/link.module).

Note: I haven't actually added any content yet.

When I try to disable the module, I get: Fatal error: Call to undefined function content_notify() in /...sites/all/modules/link/link.install on line 38
This is on a hosted environment.

Thx,
Carol

Dave: I'm working on getting that done today, thanks.

Okay - the patch from Garrett didn't apply cleanly, so I had to go back and do some of it by hand. I've got it checked into the 7 branch.

I seem to be able to create link fields, and I can enter a link into a node. The problem is that when the node is loaded, the field data isn't added to the node. I'm not sure why - the best I could figure out was that for some reason the entity types for the field was empty, where (for instance) for the body field it was 'node'.

I tried to figure out why this was blank, and didn't get anywhere good.

Awesome, thanks for getting us off started jcfiala and Garrett! I've got paid time tomorrow to start help with the port since it's a blocker on a project for work.

Dave, and whoever works on this, I just saw they've sent out a new change to the API.

(This, by the way, is part of the reason I haven't been in as much of a rush to get Link 7 up and working yet - the API doesn't seem frozen at all yet. :)

"hook_field_schema() must be moved from the .module to the .install file. This affects every module that implements fields. Please update your module. In the process, if you use t() in that function, turn it into $t()." #902264: Move hook_field_schema() to .install files

Not as important, but something that came to mind from reading the next update: "Managed File API functions like file_copy() can no longer use bare paths for filepaths, they must use URIs with public://, private://, or temporary:// (or another Drupal-provided scheme). This will probably break your module if you use file_copy(), file_move(), or the like. Check here for a posting on file API changes that have gone in recently." - It would be cool if D7's link field would link to these drupal-provided filepaths - probably a minor change to the validation function for urls that's been built up, but a useful one.

Oh, and a followup note - Currently in the 6.x branches the 'extra' fields for the link module get serialized together into a string and stuck into one field. I'm considering undoing that for d7, and making those separate fields, as serialized arrays annoy me. Currently I'm just trying to get the darn field to work before getting fancy on changing anything though. :)

Yeah let's just get it functional first. :)

@#29: Any (new?) idea what can be done to make loading the stored values work?

@jcfiala Any news on the port?

No news - Dave Reid was saying he was going to do some work on it, but I haven't heard anything from him. Meanwhile, API changes still creep over the wire, although happily I don't think any of them affect this module on the latest group of changes.

I'm trying to free up more free time to work on this.

@jcfiala: Sorry I thought I had posted my patches to help on the port. I'll get them posted here or in separate issues shortly.

subscribe

subscribe

This seems like a key field type. Wonder why it wasn't included in d7 core? Subscribe.

Sweet - thank you!

I'm afraid I'm busy the next couple of nights, but I'm going to give this a try as soon as I can.

Thank you, tristanoneil. That patch does seem to fix the issues of links not showing up in the standard display.

It looks like we still have an issue with Views when there is more than value for the links field on a node (a whole view of the node appears -- in duplicate -- one copy for each link), but I think that issue is actually in Views, not the Link module, since I tested by allowing multiple values for another field and got similar behavior in our views display. (I think this might be related to the "distinct" feature not being supported yet in Views.) For use cases which only require one link (or zero to one), the links module should be working fine with Views now, so this issue (which I'm pretty sure is purely in Views) only affects use cases with multiple values for the Link field.

I just patched my module, uninstalled the unpatched Link.module, deleted my old Link field, and installed the new patched module.

Added the field and tried to save a value, and received these errors (Drupal 7 Beta 1):

Notice: Undefined index: enable_tokens in _link_sanitize() (line 472 of /sites/all/modules/link/link.module).
Notice: Undefined index: title in _link_sanitize() (line 510 of /sites/all/modules/link/link.module).
Notice: Undefined index: enable_tokens in _link_sanitize() (line 510 of /sites/all/modules/link/link.module).

Tried with the "Allow user-entered tokens" on and off.

Also, unless you allow multiple values, the label for the field does not show up on my editing screen. Just a box that says URL and Title. When multiple values are being entered the label "Website" is visible.

edit: Every time I change the field settings, the saved values seem to disappear. Is this a symptom of all Fields or just Link?
I think it should be noted I am adding fields to the User bundle, not Node

Okay, I threw it on, and we've got basic functionality that's better than what we had before.

Fantastic!

I'm going to apply these changes to cvs tonight.

Looks like #41 got committed - why does this issue remain open? Fixed, right?

Wow. New theme kicked in, I see.

Where was I?

Looks like #41 got committed - why does this issue remain open? Fixed, right?

Well, not entirely. I'm hoping to get more feedback in on the basic functionality.

That said, more minor issues should probably start showing up in their own tickets - for one, the order of the fields (title and url) looks wonky to me, and I'd prefer they were side by side instead of on top of each other. There's probably others.

Also, I haven't even tried the automated tests. Some of them are probably going to turn out to have been too tied to the D6 version to run in D7, but some form of testing is needed so that as we move forward fixing small problems we don't suddenly cause any new large ones.

That said, more minor issues should probably start showing up in their own tickets.

Agreed. That's going to be easier to work off. Otherwise we get confused with what issue node id relates to what commit.