If Views is being used, the settings at admin/settings/image_gallery do nothing.

Patch to add a warning to this effect.
(Overriding the entire form seems too complicated, as the call to system_settings_form supplies buttons that then are out of place.)

CommentFileSizeAuthor
#17 image_gallery_207329.patch2.68 KBdrewish
#14 image_gallery_207329_allowdisable_adminwarn.patch2.5 KBHetta
#8 image_gallery_allowdisable_adminwarn.patch2.64 KBjoachim
#5 image_gallery_adminnote2.patch1.47 KBjoachim
image_gallery_adminnote.patch845 bytesjoachim

Comments

drewish’s picture

drewish commented

can we just conditionally display the form? if views is enabled we hide the form and just display a note saying that all the config is done via views and provide a link?

drewish’s picture

drewish commented
Status: Active » Needs review
joachim’s picture

joachim commented

The menu item callback would need to be conditional, as it's currently a 'get form'. Is that advisable -- does it create cache issues, for instance?

drewish’s picture

drewish commented

it shouldn't cause problems. enabling/disabling views module would cause a rebuild of the menu cache.

joachim’s picture

joachim commented
StatusFileSize
new image_gallery_adminnote2.patch1.47 KB

Updated patch.
Two different messages, depending on whether Views_UI is there too.

drewish’s picture

drewish commented

i'm wondering if we should check that the image_gallery view exists in addition to checking that the view module exists... that why if people want the old code they can just disable the view. what do you think?

joachim’s picture

joachim commented

I wonder what happens if people disable the view actually.
They might get no galleries at all...
I'll look into it & file a separate issue.

joachim’s picture

joachim commented
Title: Warn that image gallery admin settings have no effect » Recognize disabling of view, & warn that image gallery admin settings have no effect
StatusFileSize
new image_gallery_allowdisable_adminwarn.patch2.64 KB

Actually, I'm going to do this as one patch -- hope you don't mind, but my version is all tangled up with several things at once. I hope I've stripped out the menu tabs changes!

This patch:
- recognizes if the admin has disabled the view, and if so, uses the old built-in system (I've tested that overriding the view works properly; Views doesn't count this as disabled)
- if the view system is being used, shows a note on the galleries admin page as in the previous patch.
- since checking this is a bit of a mouthful, I've added a function image_gallery_is_using_views()

Chavor23’s picture

Chavor23 commented

Are you very certain of the result ? I think a caution is worth it.

joachim’s picture

joachim commented

Sorry, could you explain what you mean?
What result? What image_gallery_is_using_views() returns?

Hetta’s picture

Hetta commented

Marked http://drupal.org/node/214249 as duplicate.

Hetta’s picture

Hetta commented

I tried to apply the patch in #8, and got

File to patch: image/contrib/image_gallery/image_gallery.module
patching file image/contrib/image_gallery/image_gallery.module
patch: **** malformed patch at line 21: @@ -73,6 +74,31 @@

Anybody else?

Hetta’s picture

Hetta commented
Status: Needs review » Reviewed & tested by the community

OK, cleared the cache, and the patch applies, and works a treat.

Hetta’s picture

Hetta commented
StatusFileSize
new image_gallery_207329_allowdisable_adminwarn.patch2.5 KB

Ah yes, this is the patch which applies cleanly ...

... and the INSTALL.txt modification, too.

I wonder, would it be better to name the gallery views "views_gallery_default.inc", and the block views "views_blocks_default.inc"? That'd remove redundancy, for the gallery views.

joachim’s picture

joachim commented

I don't understand your patch on INSTALL.txt.
There's no need to copy anything to /includes/, and contrib modules shouldn't mess with Drupal's core folders anyway.
The two .inc files are loaded up by the image gallery module itself. I'm following drewish's practise of having code that depends on other modules separated into inc files, as he's done in image.module.

Also, what do you mean about renaming? Do you mean change the name of the files, or change the system name of the views?

Hetta’s picture

Hetta commented

Really? I've copied them over, just because. I'll delete that patch.
The renaming is moot, then, too.

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new image_gallery_207329.patch2.68 KB

how does this look?

Hetta’s picture

Hetta commented
Status: Needs review » Reviewed & tested by the community

Lovely, works a treat.

joachim’s picture

joachim commented

Isn't that patch going to produce redundant Submit / Reset buttons, because you're within the form generation code?
That was why I put it into a separate function.

drewish’s picture

drewish commented

joachim, no it shouldn't, the return $form; breaks out before the buttons are added.

joachim’s picture

joachim commented

Seems fair enough -- let's commit before it rots :)

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Fixed

done, thanks for the reminder.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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