Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't save missing close_date value as numeric zero #6

Merged
merged 1 commit into from
Nov 30, 2014

Conversation

juho-jaakkola
Copy link
Collaborator

The value was being cast to int which which caused missing value to be interpret as timestamp with value 0. Now value of the missing field is correctly null and therefore not saved at all.

Fixes #5

The value was being cast to int which which caused missing value to be interpret
as timestamp with value 0. Now value of the missing field is correctly null and
therefore not saved at all.

Fixes iionly#5
@iionly
Copy link
Owner

iionly commented Nov 26, 2014

The original code checked for the current value of $close_date in the edit action.

For new polls:

if ($close_date) {
    $poll->close_date = $close_date;
}

For editing existing polls:

if ($close_date) {
    $poll->close_date = $close_date;
} else {
    if (!empty($poll->close_date)) {
        $poll->deleteMetadata('close_date');
    }
}

This should allow for changing a poll with a close date to an unrestricted poll again and removing the metadata if again if the close date has been removed.

I don't know if the original code ever worked (I might not have tested removing a close date before). Currently, it fails to work but this might be due to a bug in Elgg 1.9.5 (I don't know if it has already been reported but it seems empty form fields fail to work on Elgg 1.9.5).

I've not yet tested your new code. But it should allow for removing a closing date. I don't know if this works without the if-clause.

@juho-jaakkola
Copy link
Collaborator Author

There's no need for the if clause. If metadata value is set to null, the metadata will not be created (and an existing metadata will be deleted.)

$entity->close_date = null;

The datepicker input however has the bug, that doesn't allow you to remove the value once it has been saved. So therefore removing the close date has never worked.

@iionly
Copy link
Owner

iionly commented Nov 27, 2014

Good to know about the removal of metadata when null gets assigned. I wasn't aware of that.

Yep. Removal of closing date didn't work with the old code either (apparently never tested this...).

Do you know if the datepicker input issue with removal of date will be fixed in core? Otherwise it might be an option to add some way of removing the closing date to the edit poll form.

@juho-jaakkola
Copy link
Collaborator Author

Yes, it is something that should be fixed in the core. I don't know when, though.

@iionly iionly merged commit 0896015 into iionly:master Nov 30, 2014
@juho-jaakkola juho-jaakkola deleted the issue-5 branch November 30, 2014 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Poll closing date not correctly handled anymore
2 participants