xoops forums

zyspec

Module Developer
Posted on: 2013/5/21 15:59
zyspec
zyspec (Show more)
Module Developer
Posts: 1077
Since: 2004/9/21
#11

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

@irmtfan,
Quote:

I cannot see any need or any mean (do you see any?) to have more than one xoopspoll integrated with newbb so IMO adding a new field to "bb_topics" for this purpose is just making more unneeded queries.

I don't see the need to have newbb use more than one poll module at a time. The thought behind using the 'mid' (or as you suggest 'dirname') is just to make sure the poll is associated with the poll module that is being used by newbb.

I don't understand your concern about additional queries. All it should take is adding something like $criteria->add(new Criteria('poll_mid', $poll_mid, '=')); to an existing query.

Quote:

The only need is let user to select the preferred xoopspoll module at the first step. once he select the xoopspoll module and create the first poll that selection should be hidden.


I'm not sure how this could be implemented effectively - but maybe you have some ideas. The number of potential installation scenarios seems pretty large. I'm not a big fan of providing an administrator an option (preference) that can be set "wrong" (those that would cause malfunction). It appears this will take some thought to get it right.

Quote:

besides in "irmtfan" branch of newbb i didnt implement any big feature until now. (eg: no field in tables) So the main question is am i allowed to make this branch more and more different with Alfred version.


This is always a hard thing to determine in an open source community - especially when the lead (Alfred) isn't readily available. You could contact alfred (or see if Mamba can contact him) to see if he still plans to actively develop newbb.

Quote:

IMO dirname is better than mid because mid is not consistent in all installations.

I agree that it's probably best to use something other than 'mid' due to portability between installations, although I believe this would potentially occur less than an admin inadvertently changing a Preferences setting.

You could find the poll module(s) by looking for a 'signature' (file name, class, etc.) that is consistent between poll modules.

redheadedrod

Home away from home
Posted on: 2013/5/21 18:12
redheadedrod
redheadedrod (Show more)
Home away from home
Posts: 1296
Since: 2008/2/26
#12

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

Should check and see what type of performance difference there is between MID and DIRNAME.

You can always store the DIRNAME within the settings table but use the MID for any processing. Whenever you do an Update on the module then you make sure a method is run that resets the MID to the proper DIRNAME. Or any time you change the poll module you match up this value again. This would take care of any issues with moving it between systems. You would just need to run update on the module and it would repair its self.

As to the option of changing poll modules...
What happens now if you remove the poll module? Does newbbs recognize this?

I would likely just set it up to prevent changing of the poll module if there are any visible polls. You could provide a list of those polls that would be affected by changing the polls in newbbs. I don't think blindly blocking access will be very user friendly. And providing a list is a good way to show an admin what they need to do to make the swap.

You could also maybe provide a "transfer" option within newbbs to allow any polls to be moved to a different poll program as long as it is supported by newbbs. I would agree that only supporting one poll module at a time would be a good thing.



zyspec

Module Developer
Posted on: 2013/5/21 23:44
zyspec
zyspec (Show more)
Module Developer
Posts: 1077
Since: 2004/9/21
#13

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

I'm not sure what umfrage does but this version of xoopspoll will uninstall itself 'gracefully'. Xoopspoll will remove the poll and all associations in newbb if there are polls associated with a newbb topic.

You can tell which polls are associated with a newbb topic in xoopspoll administration panel.

irmtfan

Module Developer
Posted on: 2013/5/22 8:04
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#14

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

Quote:
I don't understand your concern about additional queries. All it should take is adding something like $criteria->add(new Criteria('poll_mid', $poll_mid, '=')); to an existing query.
i think in any case a new field means more time/load in select, update , ... 1- in existing query we have to modify all existing queries. needs time. 2- sometimes we should add new query. If we use dirname or mid field we should drop config "poll_module" so sometimes we should recheck to find if poll_module is still exist or active. for example when i want to delete a poll after delete the topic. now i just write a standalone function.
public function deletePoll($poll_id)
    {
        if (empty(
$poll_id)) return false;
        
$module_handler = &xoops_gethandler('module');
        
$newbbConfig newbb_load_config();
        
$pollModuleHandler =& $module_handler->getByDirname($newbbConfig["poll_module"]);
        if (!
is_object($pollModuleHandler)  || !$pollModuleHandler->getVar('isactive')) return false;
        
// new xoopspoll module
        
if($pollModuleHandler->getVar("version") >= 140) {
            
$poll_handler =& xoops_getmodulehandler('poll'$newbbConfig["poll_module"]);
            if (
false !== $poll_handler->deleteAll(new Criteria('poll_id'$poll_id'='))) {
                
$option_handler =& xoops_getmodulehandler('option'$newbbConfig["poll_module"]);
                
$option_handler->deleteAll(new Criteria('poll_id'$poll_id'='));
                
$log_handler =& xoops_getmodulehandler('log'$newbbConfig["poll_module"]);
                
$log_handler->deleteAll(new Criteria('poll_id'$poll_id'='));
                
xoops_comment_delete($GLOBALS['xoopsModule']->getVar('mid'), $poll_id);
            }
        
// old xoopspoll or umfrage or any clone from them
        
} else {
            
$classPoll $this->loadOldPoll();
            
$poll = new $classPoll($poll_id);
            if ( 
$poll->delete() != false ) {
                
$classOption $classPoll "Option";
                
$classOption::deleteByPollId($poll->getVar("poll_id"));
                
$classLog $classPoll "Log";
                
$classLog::deleteByPollId($poll->getVar("poll_id"));
                
xoops_comment_delete($xoopsModule->getVar('mid'), $poll->getVar('poll_id'));
            }
        } 
// end poll_module new or old
        
return true;
    }
but if we drop config i have to check the poll_module field. I think about dropping topic_haspoll field. this field is useless we can just have a poll_id. if poll_id > 0 => topic has a poll I dont understand why old developers add this field. Quote:
I'm not sure how this could be implemented effectively - but maybe you have some ideas.
for sure! see newbb/xoops_version.php
// START irmtfan add a poll_module config
$pollDirs = array();
$dir_def 0;
// if in install, update
if($isModuleAction) {
    
$allDirs xoops_getActiveModules();
    foreach(
$allDirs as $dirname) {
        
// pollresults.php file is exist in all xoopspoll versions and umfrage versions
        
if(file_exists($GLOBALS['xoops']->path("modules/" $dirname "/pollresults.php"))) {
            
$pollDirs[$dirname] = $dirname;
        }
    }
    
// priorities for default poll module : 1- xoopspoll 2- last element in array 3- if no poll module => 0
    
$dir_def = !empty($pollDirs) ? (!empty($pollDirs["xoopspoll"]) ? $pollDirs["xoopspoll"] : end($pollDirs))
                                 : 
0;
}

$isPref = (
    
// action module "system"
    
is_object($GLOBALS["xoopsModule"]) && "system" == $GLOBALS["xoopsModule"]->getVar("dirname""n")
    &&
    
// current action
    
$_REQUEST['fct'] == "preferences"
    
);
// if in pref AND click on save AND 'poll_module' != 0
if($isPref && !empty($_POST['poll_module'])) {
    
$hModConfig xoops_gethandler('config');
    
$criteria = new CriteriaCompo();
    
$criteria->add(new Criteria('conf_name'"poll_module""="), "AND");
    
$criteria->add(new Criteria('conf_formtype'"select""="), "AND"); // not hidden
    
$criteria->add(new Criteria('conf_id'"(" implode(", ",$_POST['conf_ids']). ")""IN"), "AND");
    
$pollOptions $hModConfig->getConfigs($criteria);
    
$pollOptions end($pollOptions);
    if(
is_object($pollOptions) && $pollOptions->getVar("conf_value") != "0") {
        
$topic_handler xoops_getmodulehandler('topic'$modversion['dirname']);
        
$topicPolls $topic_handler->getCount(new Criteria("topic_haspoll"1));
        if(
$topicPolls 0) {
            
$pollOptions->setVar("conf_formtype""hidden");
            
$result $hModConfig->insertConfig($pollOptions);
            if(!
$result) {
                
//echo "error: poll_module is in danger!!!";
            
}
        }
    }
}
xoops_loadLanguage('admin'$modversion['dirname']);
$i count($modversion['config']); // temporary until change the whole xoops_version config
$i++;
$modversion['config'][$i]['name'] = 'poll_module';
$modversion['config'][$i]['title'] = '_AM_NEWBB_POLLMODULE';
$modversion['config'][$i]['description'] = '_AM_NEWBB_POLLMODULE';
$modversion['config'][$i]['valuetype'] = 'text';
$modversion['config'][$i]['default'] = $dir_def;

if(
count($pollDirs) <= 1) {
    
$modversion['config'][$i]['formtype'] = 'hidden';
} else {
    
$modversion['config'][$i]['formtype'] = 'select';
    
$modversion['config'][$i]['options'] = $pollDirs;
}
// END irmtfan add a poll_module config
and $isModuleAction is:
$isModuleAction = @(
        
// action module "system"
        
!empty($GLOBALS["xoopsModule"]) && "system" == $GLOBALS["xoopsModule"]->getVar("dirname""n")
        &&
        
// current dirname
        
($dirname == $_POST["dirname"] || $dirname == $_POST["module"])
        &&
        
// current op 
        
("update_ok" == $_POST["op"] || "install_ok" == $_POST["op"] || "uninstall_ok" == $_POST["op"])
        &&
        
// current action
        
"modulesadmin" == $_POST["fct"]
        );
That is reliable way. - in update and install it will check and add config. if there is just one poll the config will be hidden. if there is more than one poll module it will select with this priority: 1- xoopspoll 2- last element in array ps: we can leave all OnInstall, OnUpdate and OnUninstall functions and use this method. It will be very useful to make a module like xoopspoll clonable with a simple rename of dirname. even it will be possible to update a clone "blabla" using original "xoopspoll" - when user goes to pref and click on submit. it will check the "poll_module" config and find if there is a topic with poll. then it will hide the config. Note: currently it is not possible to find the poll module in newbb installations before most recent version (version 4.32) but we can add some guess to the above code. Quote:
if he still plans to actively develop newbb.
AFAIK Alfred continue developing newbb but some adding/dropping features are not acceptable for me. eg: he dropped the user moderation in forums and replace it with group moderation. It is very bad. I need to let one user to be a moderator in one forum without any additional group or permissions. Quote:
You could find the poll module(s) by looking for a 'signature' (file name, class, etc.) that is consistent between poll modules.
I used pollresults.php. it is consistent in all xoopspoll and umfrage versions and all current and future clones. do you have better idea? you used hard-code date() function in the current xoopspoll 1.4. do you want to consider using formatTimeStamp or better XoopsLocal::formatTimeStamp for formatted times in templates? in result forms you should use it. but in edit forms we still should add the hejira date in JQuery date time picker @redheadedrod: Quote:
What happens now if you remove the poll module? Does newbbs recognize this?
because newbb and all other poll modules are completely separated modules we cannot add many tight relations. but newbb will check the poll module existence every time wants to do something. but if somebody uninstall xoopspoll newbb will not understand it. (that is why zyspec add the functionality to xooppoll) I think xoopspoll will not understand whether newbb is uninstalled too

zyspec

Module Developer
Posted on: 2013/5/22 13:31
zyspec
zyspec (Show more)
Module Developer
Posts: 1077
Since: 2004/9/21
#15

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

Thanks irmtfan. I will read through your latest response more closely and see what, if anything, I need to do to xoopspoll 1.40 to improve it. For example, I'll have to go look for the 'hard coded date() functoin' usage - I thought I'd removed those where applicable.

I know I'll have to write some code specifically for the 4.32 newbb changes you've made. Previously newbb was written so that I could assume that any poll associated with newbb was for xoopspoll so I could just delete the poll info in newbb. That's no longer true (could be a clone of xoopspoll or umfrage). I'll have to create a function to determine if the polls are associated with 'this' copy of xoopspoll and then take the appropriate actions.

sabahan

Quite a regular
Posted on: 2013/5/22 14:23
sabahan
sabahan (Show more)
Quite a regular
Posts: 317
Since: 2006/3/4 5
#16

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

thank you to all for this new version

irmtfan

Module Developer
Posted on: 2013/5/23 2:06
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#17

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

@zyspec : As you can see i used (copy/paste) many of your codes for xoopspoll 1.4 functionality in newbb located in xoopspoll/extras/newbb_4x/irmtfan (so you can remove that extra in your next versions) Therefore thanks to your codes, I didnt have to put time to learn your xoopspoll 1.4 functionality. Of course there were some bugs which i solved. so it was a good team work. After that I decided to make newbb work with any poll module regardless of version or dirname so i did that too. Quote:
That's no longer true (could be a clone of xoopspoll or umfrage). I'll have to create a function to determine if the polls are associated with 'this' copy of xoopspoll and then take the appropriate actions.
Yes previously it was hardcoded to "xoopspoll" Today i add a function to find poll module that is in used in the current newbb installation. This function is totally reliable and you can use it. so you dont need to put time on it.
// START irmtfan findPollModule
    /**
     * find poll module that is in used in the current newbb installtion.
     * @access public
     * @param array $pollDirs dirnames of all active poll modules
     * @return string $dir_def | true | false
     *    $dir_def: dirname of poll module that is in used in the current newbb installtion.
     *    true: no poll module is installed | newbb has no topic with poll | newbb has no topic
     *    false: errors (see below xoops_errors)
     */
    
public function findPollModule($pollDirs = array()) {
        if(empty(
$pollDirs)) $pollDirs $this->getActivePolls();
        if(empty(
$pollDirs)) return true;
        
// if only one active poll module still we need to check!!!
        //if(count($pollDirs) == 1) return end($pollDirs);
        
$topicPollObjs $this->getAll(new Criteria("topic_haspoll"1));
        if(empty(
$topicPollObjs)) return true// no poll or no topic!!!
        
foreach($topicPollObjs as $tObj) {
            
$poll_idInMod 0;
            foreach(
$pollDirs as $dirname) {
                
$pollObj $tObj->getPoll($tObj->getVar("poll_id"), $dirname);
                if(
is_object($pollObj) && ($pollObj->getVar("poll_id") == $tObj->getVar("poll_id"))) {
                    
$poll_idInMod++;
                    
$dir_def $dirname;
                }
            }
            
// Only one poll module should has this poll_id
            // if 0 there is an error
            
if($poll_idInMod == 0) {
                
xoops_error("Error: Cannot find poll module for poll_id='{$tObj->getVar('poll_id')}'");
                return 
false;
            }
            
// if 1 => $dir_def is correct
            
if($poll_idInMod == 1) {
                return 
$dir_def;
            }
            
// if more than 1 continue
        
}
        
// if there is some topics but no module or more than one module have polls
        
xoops_error("Error: Cannot find poll module that is in used in newbb!!! <br><br>You should select the correct poll module yourself in newbb > preferences > poll module setting.");
        return 
false;
    }
    
// END irmtfan findPollModule
so the usage is as simple as this:
$topic_handler xoops_getmodulehandler('topic''newbb');

            
$poll_module_in_use $topic_handler->findPollModule();
as you can see the output is informative: Quote:
* $dir_def: dirname of poll module that is in used in the current newbb installation. * true: no poll module is installed | newbb has no topic with poll | newbb has no topic * false: errors (see below xoops_errors)
I used that in 'OnUpdate' and 'preferences saving' processes to make sure the "poll_module" config will be saved correctly. I changed the version to newbb 4.33 and RC7: http://svn.code.sf.net/p/xoops/svn/Xo ... b/branches/irmtfan/newbb/ Quote:
the 'hard coded date() functoin' usage - I thought I'd removed those where applicable.
yes. eg: in xoopspoll/class/renderer.php line 173 should be replaced by this
$xuEndFormatted formatTimeStamp($xuEndTimestamp);
I think it would be the best solution for you to write a helper class and be free from any dirname, config, handler, date format, ... in next version of xoopspoll helper really reduce my coding time in userlog.

zyspec

Module Developer
Posted on: 2013/5/23 2:47
zyspec
zyspec (Show more)
Module Developer
Posts: 1077
Since: 2004/9/21
#18

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

@irmtfan,

I'll have to duplicate something similar to your findPollModule() method so xoopspoll will work with the newbb trunk code too - until you are able to merge your changes into the trunk.

I'll look at using formatTimeStamp although I've had problems with it in the past - especially supporting DST correctly when the server is in a different timezone.

Side Note: What's needed is for XOOPS to abandon the current date/time handling routines and change date handling to use the PHP DateTime class support that was implemented back in PHP 5.2 - but this is a pretty big rewrite of how dates are handled in XOOPS and probably won't be changed any time soon.

irmtfan

Module Developer
Posted on: 2013/5/23 6:35
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#19

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

Quote:

I'll have to duplicate something similar to your findPollModule() method so xoopspoll will work with the newbb trunk code too - until you are able to merge your changes into the trunk.


I forgot to mention that.
instead of all other modules in svn, newbb trunk is very old, full of bugs and IMO bad working version and it seems nobody is using this version now. So leave newbb trunk and just stick on 2 versions:
1- Alfred newbb 4.3 stable version (of course still has many bugs) and maybe newbb 4.4
http://www.simple-xoops.de/projekt/2:1-Forum_-_newBB.html

2- irmtfan newbb 4.33 RC7

but to support Alfred and irmtfan < version 4.33 you dont need to write codes because newbb Alfred amd irmtfan < 4.33 only support hard-coded "xoopspoll" and "umfrage" dirnames and not any clone.
to recognize irmtfan version >= 4.33 you can use "author":
$modversion['author'] = "Marko Schmuck (predator) / D.J. (phppp) / Alfred(dhcst) / xoops.org (irmtfan)";
or i can add another special recognition.

IMO and based on tests, current irmtfan branch is the most stable newbb version. I found and solved many bugs in newbb1, newbb2, newbb 3.08 and ...

irmtfan

Module Developer
Posted on: 2013/5/24 2:57
irmtfan
irmtfan (Show more)
Module Developer
Posts: 3419
Since: 2003/12/7
#20

Re: Xoopspoll v1.40 Beta 1 - Ready for testing

sorry forget what i wrote in the last post about the way to find newbb version.
you can simply use method_exists to find if it is the newbb version which support any poll module.
$topic_handler xoops_getmodulehandler('topic''newbb'); 
if (
method_exists('NewbbTopicHandler''findPollModule')) {
    
$poll_module_in_newbb $topic_handler->findPollModule();
} else {
    
$poll_module_in_newbb "xoopspoll"// it is hard-coded no clone is supported
}

if(
$poll_module_in_newbb == $thisModule->getVar("dirname") ) {
 
// do some actions like uninstall, ...
}

So IMO you dont need to write any extra code because as i said all old newbb versions only support hard-coded "xoopspoll" dirname.

Note: we should use the above findPollModule only in OnInstall, OnUpdate and OnUninstall process because it is an in depth checking to find the newbb poll module.
In common usages you can search for and use new config "poll_module" in newbb.

Quote:

until you are able to merge your changes into the trunk.

Technically merging any old newbb version < 4.4 Alfred (include newbb 2, newbb 3.08 and trunk) with the latest irmtfan branch is as simple as copying files.
But newbb needs a team of developers include at least one core developer and an official badge.

Quote:

change date handling to use the PHP DateTime class support that was implemented back in PHP 5.2


By adding any enhancements in the future, xoops core will always support its own function "formatTimestamp" or method "XoopsLocal::formatTimestamp"
So all developers should use it now and their modules will be work in any newer version of xoops.

Edit:
A side note:
this line will be dangerous for the current cloning method in xoopspoll using smartclone:
$poll_module_in_newbb "xoopspoll"// it is hard-coded no clone is supported

because it should be remained as "xoopspoll" in all xoopspoll clones!!!
That is show the necessity of using a helper class and try to remove any instance of dirname (here: xoopspoll) from codes as much as possible.
so in the clone process you dont need to change strtolower($dirname) anymore.