Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

I plan to do this, but I can't do it until next week at the earliest... provisionally assigning this to myself but if someone else wants to take it on, feel free.

Senpai’s picture

Issue tags: +8hr
twardnw’s picture

Assigned: jhodgdon » twardnw

Assigning to me :) Soon as we have a database snapshot from 6->7 that catches feature (content type, et al)

jhodgdon’s picture

Is that supposed to mean you expect this task to take 8 hours?!? I would guess 1 at the most. It's about 10 lines of code very similar to others that have already been ported, and a few lines of CSS to be added to Bluecheese, and a feature to recreate in 7.

Senpai’s picture

Jen, it'd be 1hr if you were doing it, but since you're not, it's one calendar day, give or take.

jhodgdon’s picture

I offered to do this particular upgrade, as part of the contrib project of getting this feature on d.o, after it was deployed. Still happy to do it, just let me know, but twardnw claimed it so I stepped aside.

Senpai’s picture

@jhodgdon, I'd love it if you could tackle this issue! But, can you do it today? If so, great! Claim it for yourself and have at it. If not, I understand, and I'll have @twardnw do it today.

jhodgdon’s picture

I can take care of it today if there is an updated dev server to work on. Let me know.

jhodgdon’s picture

Assigned: twardnw » jhodgdon

I'm working on this now.

jhodgdon’s picture

So... I went to the d7 test site for this, and it looks like:
- The crosssite patch from #1487988-176: Create new content type and Views for books was applied to D7 already, and it is working.
- Some of the drupalorg_handbook patch from the same place was applied; some wasn't. Some of what was applied was done in a D6 manner and needed a bit of adjusting.
- The content type is missing (it might be there in the DB but it is disabled).
- The View is missing.
- The feature is missing.

Anyway... Here's a preliminary patch for drupalorg_handbook which I'm attaching here so I can test this much of it. If this much works, it will still need a patch for the drupalorg_handbook_preprocess_content_field() function at the end of the drupalorg_handbook.module file.

I'm also going to try taking the d6 exported feature from the other issue and see if it will read OK in D7. It should be a start...

jhodgdon’s picture

That drupalorg patch had a typo, so here is a new one. It still needs a patch for the drupalorg_handbook_preprocess_content_field() function at the end of the drupalorg_handbook.module file, which is the part that turns the ISBN field into a purchase link on Amazon.

I also managed to get the feature updated, so here is a D7 export of the feature.

Some styling is also needed, but that is a separate issue (#1836254: Theme Book listing pages and view).

jhodgdon’s picture

Here's a slight tweak to the feature: made the view hide empty fields, and made the content type accept GIF images.

jhodgdon’s picture

And here's a new drupalorg patch to fix an undefined array element warning.

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
FileSize
9.72 KB

And one more update for the feature, to include the image. It probably needs a new image style for the books page.

I'm out of time... to dos:
- Styling on that other issue (needs CSS expert to overcome the clearfix nonsense)
- Image style for the book view page
- Fix the drupalorg_handbook_preprocess_content_field() function at the end of the drupalorg_handbook.module file

jhodgdon’s picture

A note on the preprocess function... it probably needs to be
http://api.drupal.org/api/drupal/modules!field!field.module/function/tem...
in other words, rename it to drupalorg_handbook_preprocess_field() and tweak the code a little for d7 fields vs. d6 cck

jhodgdon’s picture

Status: Active » Needs review
FileSize
5.36 KB

OK, I have the preprocess function done and it works too.

So the only remaining ToDo on this issue is probably an image style, which I guess probably gets added to the feature?

Anyway, here's a new handbook patch.

jhodgdon’s picture

I checked the old d6 test site and the image is formatted with style "grid-2" there. Applied that style and it looks good! So here is an export of the feature, and I think this issue is done! (once the patches are committed).

Sorry about all the comments... What needs to be committed is the patch in #16 for drupalorg, and then this feature export needs to be added to drupalorg/features too.

jhodgdon’s picture

Dang. One more patch for drupalorg to fix a PHP warning.

drumm’s picture

Status: Needs review » Needs work

I committed #18 and followed up with a couple code style changes: http://drupalcode.org/project/drupalorg.git/blobdiff/e8f5c110d1903670f73...

I'm not committing the feature yet since it takes ownership of all 6 vocabularies associated with book listing content types. Audience, Keywords, and Level are also used by book pages; and Drupal version is also used by a few types. Either:

  • Feature modules should not own these. Not sure if this is possible, they might be dependencies for the taxonomy fields, which we do want to be owned by this feature.
  • There should be a separate feature module for all Drupal.org vocabularies, this feature would depend on that. Or separate features which categorize things better than lumping them all into the same feature.
jhodgdon’s picture

FileSize
701 bytes

Here is a feature with just the taxonomies... still testing, but I need this somewhere so I can download it and test it out.

jhodgdon’s picture

OK, ignore #20. Here are two features. One has the 6 taxonomies only, and the other has the other stuff for book listings, and depends on the taxonomy feature.

drumm’s picture

Status: Needs work » Fixed

Looks okay, committed.

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

Anonymous’s picture

Issue summary: View changes

adding 1836254

  • Commit 4f5ce65 on 7.x-3.x, 7.x-3.x-dev authored by jhodgdon, committed by drumm:
    [#1836252] D7 features for book listings
    
  • Commit e8f5c11 on 7.x-3.x, 7.x-3.x-dev by drumm:
    [#1836252] Update book listings for D7