Comments

mordonez’s picture

Oh, please response for this request.
I'm waiting with impacience this modules for drupal 5

Thanks

yngens’s picture

subscribe

marcor’s picture

This would be very interesting for me, too! :)
It looks like a very nice module and could be important for my site.
I would like to add the settings code then, but for some time I'm only working on Drupal5, so a port to 5.x would be very appreciated! :)

Thank you!

marcor’s picture

Title: Module compatibility with 5.x? » Make module compatible with 5.x
Category: support » task
Status: Active » Needs work
StatusFileSize
new105 bytes

Most important thing you need to be compatible with 5.x is a file birthdays.info in the birthdays directory. I added the attached file and the birthdays module seems to work mostly. Problems on my site seem not to rely on the upgrade to 5.x but on some configuration issues:

  • helptext or configuration, because currently it needs "My Profile"-section in profile
  • sometimes (seldom) a saved date is zeroed again
  • starsigns invisible because the path needs to be "drupalized"
  • drop starsigns by configuration (do you _really_ believe this??)

With this attached file (rename to birthdays.info) you can try the module in 5.x. The state of the file is "needs review / ready to be committed", but I think some other work on the module would be a good idea.

drazzig’s picture

Hi

I'm currently travelling around a lot so sorry for not following up on this sooner. I've hardcoded the profile category so you can only change it in the code, but will definately put it as a configuration option. As for the starsigns, no don't really believe this stuff, but I thought if would be a nice feature, and for ease of initial release, hardcoded that as well. I will add it as a configuration option as well!

I'll also add the info.txt file to the build as soon as I return to the UK. Sorry for the slowness in pushing this, loads of stuff happening at work at the moment, I'm happy to accept patches from anyone who has time to work on this as well!

maartenvg’s picture

I'm making a patch at the moment that will convert it to 5.1. I also add some features (a admin settings page, making things configurable as much as possible) and look at the code for bugs and performance enhancements.

maartenvg’s picture

It's as good as finished. Later today (or tomorrow) I'll generate the patch file and attach it to this thread. I've fixed a few bugs (mostly in the SQL), made some optimizations and made most things configurable and translatable. Dates are localized and I made more use drupal's build-in functions.

I also added a new block to show the birthdays of the next N days in stead of the N number of upcoming birthdays. The idea behind this is with the original block, you can't garantee that everyone having their birthday on the same day are shown. If you show the upcoming 10 birthdays, and on 1 day there are 15 users having their birthday, 5 of them aren't show, which is, in my opinion, not acceptable. Side effect is of course that with the new block you can have days when there are no birthdays.

maartenvg’s picture

Assigned: Unassigned » maartenvg
Status: Needs work » Needs review
StatusFileSize
new25.7 KB

Here it is. It's 5.1 and working!
Everything I posted about above is implemented.
Please review the code, test it and report back if it needs any improvements. Drazzig, I hope you can implement this patch in a subsequent version and I hope you don't mind I fixed some bugs and introduced a few features/options.

Btw, things I didn't do:
- I hardly touched the Postcard-code, so I can't tell you if it still works. It's therefore not on the settings page, but if someone does indeed uses this feature and wants me to look at it: just ask, I'll see when/if I have some time for it.
- I just saw that I didn't implement the second block as I should've (although it works fine), leaving some duplicate code.
- The date of birth isn't asked during the registration process.

maartenvg’s picture

StatusFileSize
new25.74 KB

slightly altered patch-file (now relative to Drupal Root), I'm a bit new to creating patches.

yngens’s picture

maartenvg, is it possible to list all the users with the same zodiac? For instance, if I go to /birthdays/aries is it possible to see all the aries?

maartenvg’s picture

great idea, I'll check it out.

maartenvg’s picture

StatusFileSize
new3.29 KB

That wasn't hard :)
About the patch:
- it is a patch based on the 5.x version of this module. Be sure to patch 4.7 first with the other patch, /then/ use this patch for additional functionality.
- go to ?q=birthdays/pisces to show all users that are pisces, and so on.
- Starsigns are now translatable, which makes it possible to go to for example ?q=birthdays/vissen if you translated pisces to 'vissen' (which is plural for fish in Dutch). The title-attribute of the starsign-icon that is show next to the user also shows the translated starsign.

Please test this extensive against you userbase, as my development userbase is very small (1 user at the moment ;))

Have fun!

maartenvg’s picture

StatusFileSize
new3.28 KB

small update (that darn filename again)

yngens’s picture

StatusFileSize
new20.57 KB

Thank you very much, maartenvg. Now everything seems to work, except that instead of month names three ??? question marks are shown in the blocks. But month names are displayed normally when I switch from Russian to English language. I guess there are some problems with encoding or smth. There was another issue on line 239. I have changed

$blockContent .= '<ul><li class="leaf">' . l('More...', 'birthdays') . '</li></ul>';

to

$blockContent .= '<ul><li class="leaf">' . l(t('More...'), 'birthdays') . '</li></ul>';

Another thing I did not liked that sign pictures are linked to yahoo horoscope web page. I believe it is not nice to force Drupal users to go outside websites. So I have made some changes so that sign pictures now lead to the list of users of the same sign. Module file has been enclosed. I offer to continue from this file.

maartenvg’s picture

Thanks for testing the patch and catching the missing t(). I'm glad some besides me is actually using it :)

About the yahoo-connection: that was something the module owner put in, so I didn't feel like removing it. I therefore made it optional, but maybe not optional enough. I propose to add an option to determine the way the starsigns are shown (with a dropdown select): "no starsigns", "images but no link", "images & link to filtered userlist" and "images & link to yahoo". This way people who want to use the yahoo connection, can still use it (as your option is forcing them NOT to use it, and forcing is in general not a good idea I believe).

I'm not sure it's up to us to make that change, let's wait if the module owner has something to say about it. I therefore also propose not to continu based on your .module untill the owner gave his blessing to your changes as it is a decrease in functionality.

Rest us the issue of the Russian dissapearing text:
Please try to replace all (prob around 412, 427 & 457):
$month = strftime('%b',mktime(0,0,0,$user->m,1,2000));
with:
$month = format_date(mktime(0,0,0,$user->m,3,2000), 'custom', 'M');

That should do it, hopefully.

BTW, could you, next time, please generate a patch, that way it's easier for us to check what has been changed compared to including your entire *.module. It also makes the changes more compatible with people who made changes to their module. I would really apreciate it.

yngens’s picture

Dear maartenvg,

Thank you for explanations. I totally agree with your suggestions. The problem about generating patches for me is that I do not know how to do it. And since I am using Drupal for Russian-language webportal, I adapt and changes the modules a lot. So very often I can not contribute to the general stream, but only follow it.

denney’s picture

Here's a puzzle that has been bugging me for awhile now...

I applied the file in #14 and now the DOB on the user profile pages are a day behind... eg. My birthday is the 12 of Jan but it displays on the profile page as the 11 of Jan.

Sidebar blocks are fine, they say 12. The birthdays list page though says 11.

If I comment out line 287 and replace it with:
$content .= $birthday->m . "." . $birthday->d . "." . $birthday->y

it outputs "1.12.1984" so that database data is fine. It must be the call to either "mktime()" or "format_date()". I have NO IDEA what is going on here...

maartenvg’s picture

format_date uses the user's timezone to determin dates, I think (haven't tested) that it's because the mktime uses the time 0:00:00 mm-dd-year ... if your timezone is negative dateformat changes the date to for example -3:00:00 mm-dd-year which is 1 day before the actual date.
It's something I didn't take into account.

Please try to replace all mktime(0,0,0,x,y,z) in to mktime(13,0,0,x,y,z), an report back whether that helped.

yngens’s picture

i'll try suggested changes, but for now i have noticed that birthdays occasionally drop to 1 january 1970. this disappoints very much.

denney’s picture

That works perfectly. All my birthdays on the main website are fine now.

Also tested this with a website that had the right birthdays to begin with and it didn't interfere with them at all.

PieterPan’s picture

The mktime(13,...) works for me.
What I did initially to get the problem was set the timezone to 0:00, and look at my profile. Database date was indeed correct. Snegny, it would help if you could give the birthday you set and the time zone.

maartenvg’s picture

we should probably change the check to a non-timezone dependent version. When I have the time, I'll look into an improvement.

yngens’s picture

mktime(13, also worked for me. but i have other serious issues.

1. Birthday dates occasionally changes. For instance, I set my birthday several days ago and was checking it everyday. Today for some reason it changed to 1 January 1900.

2. Birthday field do not appear on the registration page. This could be because I changed "Profile category" on the module's setting page. If this is true, then the code has to be rewritten to enable Drupalers of other languages then English to change this field without harming such functionalities as displaying birthday field on registration page.

marcor’s picture

Wrong (zeroed) savings comes from the fact that you save a part of the profile where there's no birthday field.

Workaround: On my local installation I used another form from profile module ('profile_birthdate') and changed the user hook that it does not check the profile section if it is the user registration form (this is case 'insert'):

function birthdays_user($op, &$edit, &$user, $category = null) {
  switch($op) {
    case 'update':
      if($category != t('My Profile')) break;
    case 'insert':
      //if($category != t('My Profile')) break;
      $year = $edit['profile_birthdate']['year'];
      $month = $edit['profile_birthdate']['month'];
      $day = $edit['profile_birthdate']['day'];
      ...
}

But I agree, I solution in the module itself would be a smarter one.

yngens’s picture

Markour, I got that I need to create new profile page, but can I just add this peace of code to birthdays.module file or somewhere else?

marcor’s picture

These changes would have to me made to the birthdays.module. Look for the function there...

Due to the fact that birthdays module also creates a birthday ("DOB") field, you would have to comment out those parts of the form to suppress displaying this. This all is done in the same birtday_user function. Comment out case 'view' and case 'form'.

function birthdays_user ...
  ...
  case 'view':
/* $query = "SELECT birthday FROM {dob} WHERE uid = $user->uid";
  ...
*/
  case 'form':
/*
  ...
*/

But maybe a check in a settings option "Which profile field of type date contains the age?" [profile_DOB] as in validateage.module could make it possible to use birthdays module without admin's intervention if the admin has a site already running having a birthday field already defined.

yngens’s picture

yep, it would be much more nicer to be able to select options on the settings page.

denney’s picture

Ok, now I'm confused...

What exactly needs to be applied from this issue to make Birthday's compatible with 5.1?

Is there any progress on creating a 5.1 release?

maartenvg’s picture

StatusFileSize
new29.96 KB

@denney: there is progress: unofficial that is (as I'm not the maintainer but did made the patch).

Included is a 'final-so-far' patch agaist CVS (4.7) that incorporates fixes for all issues mentioned above (including issue 1 and 2 of snegny). It's now also timezone independent.

Including the option to use a profile.module date-field might pose problems as birthdays.module uses a separate table for its birthdays. When an existing profile.module field is referred to as the field birthdays.module should use, the profile.module data has to be transferred to the birthdays table somehow, because otherwise users wouldn't be shown on the lists. Another 'problem' is that the same data will be put in 2 different database tables.
Although it would indeed be a more logical choice to make birthdays.module dependent of profile.module, it would need some major changes in the code. I'm not against this, but we should discuss whether this is really necessary? Maybe we should open a new issue for this, to keep this one more focused on a 5.x version.

maartenvg’s picture

StatusFileSize
new29.96 KB

@denney: there is progress: unofficial that is (as I'm not the maintainer but did made the patch).

Included is a 'final-so-far' patch agaist CVS (4.7) that incorporates fixes for all issues mentioned above (including issue 1 and 2 of snegny). It's now also timezone independent.

Including the option to use a profile.module date-field might pose problems as birthdays.module uses a separate table for its birthdays. When an existing profile.module field is referred to as the field birthdays.module should use, the profile.module data has to be transferred to the birthdays table somehow, because otherwise users wouldn't be shown on the lists. Another 'problem' is that the same data will be put in 2 different database tables.
Although it would indeed be a more logical choice to make birthdays.module dependent of profile.module, it would need some major changes in the code. I'm not against this, but we should discuss whether this is really necessary? Maybe we should open a new issue for this, to keep this one more focused on a 5.x version.

denney’s picture

Thank you for the up-to-date patch.

On the issue of the profile field, I believe it would be beneficial for websites that require the user enter a birthday BEFORE registering.

In the current form, the user must enter their birthday on the registration page to register and then again to satisfy this module.

drazzig’s picture

As the module owner I have some things to add to the discussion.

Firstly, its great to see everyone, especially Maarten working on this. I'm afraid I'm working on a portals project in Thailand for a few months so have found it difficult to contribute to this. I am aware of all the original issues in the first release, I got it working for my own personal website, and decided just to release it as-is so at least other people could use it and update it. I'm happy to add Maarten as a contributor on the project if he accepts to maintain the CVS repository with the latest code instead of using patches for the updates.

Secondly, about the option to use Yahoo Horoscopes, I agree with Maarten, it should be a drop down box with the 4 options. Removing the link completely would take away a feature I thought was pretty cool which is why I added it. Maybe we can use an RSS feed from Tarot.com (I just saw a google my pages component with it) that can pull the user's horoscope onto the site without needing to leave it.

Thidly, about the Profiles module. If someone can explain to me how this would work I will be happy to consider it. The reason I created another table in the database is because the profiles module stores dates as an array, which not being an expert in SQL, I don't know how I would send a request to show me all the birthdays before a certain date, order them, or show me 6 next birthdays etc. It would be very inefficient to pull all the table into a PHP array and sort through this for large userbases, my philosphy is to make the database do all the work as this is more efficient, so storing it in a proper date field allows this to happen.

I haven't had a chance to review all the changes, to be honest I don't have the tools to generate and integrate patches to the code, so if someone could send me the latest module with all the patches or tell me how I can do it, that would be great, and I will review everything asap!

Thanks for all the feedback and hard work!

Drazzig

AjK’s picture

Maarten CVS account request has been approved and I have added him as a co-maintainer of this project based on the above comment.

yngens’s picture

good progress on this module

EgorfromMurmansk’s picture

I am sorry. How to add this patch? procedure? Sorry for stupid question! :)

maartenvg’s picture

Status: Needs review » Closed (fixed)

Well, it's ready. I've added my changes to CVS and the new development release will be activated somewhere in the next 12 hours.
From that moment bugs and feature request for that release should be announced in separate issues, because otherwise it's hard to efficiently help everyone out.

Please test it extensively, and remember that this is still in development. With enough feedback I'll be able to quickly release a stable 5.x version.

ckmj’s picture

Version: 4.7.x-1.0.x-dev » 5.x-1.x-dev
Status: Closed (fixed) » Postponed (maintainer needs more info)

Hi there, I am having a problem. Every birthday prior to 1970 defaults to dec 31 1969 everywhere except the blocks where they display correctly.

User Birthday Age Starsign
Kathy 31 December, 1969 107
Donna 31 December, 1969 107
Victoria 31 December, 1969 38
ckmj 31 October, 1972 34

maartenvg’s picture

Status: Postponed (maintainer needs more info) » Closed (fixed)

Please start a new issue when you find bugs or request features.
I've done this now for this bug.