Skip to content

Conversation

@alikon
Copy link
Contributor

@alikon alikon commented Apr 22, 2019

Pull Request for Issue #16788 ....same as #21901 for modules

Summary of Changes

some datetime fields should be nullable as default

  • publish_up
  • publish_down
  • checked_out_time

Testing Instructions

test com_contacts

Expected result

contacts should works as before

@ghost ghost changed the title [4.0][com_contact] nullable datetime fields [4.0] Contact - nullable datetime fields Apr 22, 2019
@alikon alikon marked this pull request as ready for review April 22, 2019 09:42
@zero-24 zero-24 dismissed their stale review April 22, 2019 09:43

Issues seams to be solved

@wilsonge
Copy link
Contributor

Open question (can see it both ways). Should modified date by nullable or should enforce it being date time (modified date is the created date on creation).

@alikon
Copy link
Contributor Author

alikon commented Apr 23, 2019

yes, to me too, we need to take a decision and make it consistently across the cms

@twister65
Copy link
Contributor

I have tested this item ✅ successfully on 44b90c5

with MySQL and PostgreSQL.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24675.

@wilsonge
Copy link
Contributor

wilsonge commented May 5, 2019

Unless anyone feels strongly I'd rather have this set all the time. So modified date is always set (to the created date time when the article is created)

@alikon
Copy link
Contributor Author

alikon commented May 7, 2019

ok modified field is not null now

p.s.
i hope prepared statement should be room for a new pr

@wilsonge
Copy link
Contributor

wilsonge commented May 7, 2019

There's no queries here that require prepared statements. This looks fine. Just needs some tests creating and updating contacts

@alikon
Copy link
Contributor Author

alikon commented May 8, 2019

are you sure ?
just for example components/com_contact/Model/CategoryModel.php is full of places where we can use prepared statements

$query->where('(a.name LIKE ' . $search . ')');

$query->where('a.published = ' . (int) $state);

@Quy
Copy link
Contributor

Quy commented May 15, 2019

I have tested this item ✅ successfully on 456218b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24675.

@Quy
Copy link
Contributor

Quy commented May 15, 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/24675.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 15, 2019
@zero-24 zero-24 added this to the Joomla 4.0 milestone May 16, 2019
@wilsonge wilsonge merged commit 9087899 into joomla:4.0-dev May 16, 2019
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 16, 2019
@alikon alikon deleted the patch-87 branch May 16, 2019 17:41
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jun 28, 2019
Pull Request for Issue joomla#16788 ....same as joomla#24675 for contacts

Summary of Changes
some datetime fields should be nullable as default

- publish_up
- publish_down
- checked_out_time

### Testing Instructions
test com_banners

### Expected result
banners should works as before
brianteeman added a commit to brianteeman/joomla-cms that referenced this pull request Jun 28, 2019
Pull Request for Issue joomla#16788 ....same as joomla#24675 for contacts - I really just copied the work of others (someone has to)

### Summary of Changes
some datetime fields should be nullable as default

- publish_up
- publish_down
- checked_out_time

### Testing Instructions
test com_newsfeeds

### Expected result
newsfeeds should works as before
wilsonge pushed a commit that referenced this pull request Jul 17, 2019
Pull Request for Issue #16788 ....same as #24675 for contacts

Summary of Changes
some datetime fields should be nullable as default

- publish_up
- publish_down
- checked_out_time
wilsonge pushed a commit that referenced this pull request Jul 23, 2019
Pull Request for Issue #16788 ....same as #24675 for contacts - I really just copied the work of others (someone has to)

some datetime fields should be nullable as default

- publish_up
- publish_down
- checked_out_time
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.

7 participants