I have tested this item ? unsuccessfully on 1bac2f1

After apply Patch got 404 - Component not found in Frontend on all Pages.

System information

3.8-dev
Multilanguage Site
macOS Sierra, 10.12.6
Firefox 55 (64-bit)

MAMP 4.1.1

• PHP 7.0.15
• MySQLi 5.6.35

  ---

  _{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17709.}

Did you fix the database?
I'm adding 2 new fields to the menu table

On 25 Aug 2017 8:33 am, "Franz Wohlkönig" notifications@github.com wrote:

I have tested this item ? unsuccessfully on 1bac2f1
1bac2f1

After apply Patch got 404 - Component not found in Frontend on all Pages.

This comment was created with the J!Tracker Application
https://github.com/joomla/jissues at issues.joomla.org/tracker/
joomla-cms/17709.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#17709 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALFse2tH_ok3b3KaSMEbCnmh-QIhdCVks5sbmq9gaJpZM4PCRNE
.

after Fix Database (please update Instructions) got:

There was a typo in table name fixed now, can you retest ;)

On 25 Aug 2017 8:42 am, "Franz Wohlkönig" notifications@github.com wrote:

after Fix Database (please update Instructions) got:
[image: bildschirmfoto 2017-08-25 um 08 41 51]
https://user-images.githubusercontent.com/8235763/29702372-5da8b538-8971-11e7-9851-c09436a045a1.jpg

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#17709 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALFsdH6HBgXEil1BtvLojtgjzhIsmIGks5sbmzwgaJpZM4PCRNE
.

Great new feature - awesome stuff - thanks

Two small issues

1. You can set the home menu to expired or published in the future. Should we allow that as it will break the site?
2. There needs to be a status icon change in the list so you can see if it is expired etc - as you can see in the list of modules

I have tested this item ✅ successfully on a711081

I get "Error
The file marked for modification does not exist: libraries/src/Menu/SiteMenu.php' when attempting to apply the patch.

please retry maybe some synch issue

My fault, was not on latest Joomla version! Testing now...

I have tested this item ✅ successfully on bb0208d

Tested both publish and unpublish fields. Worked as expected. I agree there needs to be a status icon in the list view.

Overall great feature! Honestly, I'm shocked this hasn't been implemented sooner.

small issues

You can set the home menu to expired or published in the future. Should we allow that as it will break the site?

No,

i've added a check for past/future when home, but not sure if it is enough for multilanguage

There needs to be a status icon change in the list so you can see if it is expired etc - as you can see in the list of modules

yes, added

p.s
wasn't sure it was a good idea ?

can you give a go if this PR is read for test?

Is ready

On 26 Aug 2017 12:52 pm, "Franz Wohlkönig" notifications@github.com wrote:

can you give a go if this PR is read for test?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#17709 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALFsSzj-SEyZr6D95RWTGBbp69mbiKPks5sb_j4gaJpZM4PCRNE
.

I have tested this item ✅ successfully on 9f57098

1. Any reason for tests/unit/schema/sqlsrv.sql to have windows EOL (that was also before patch)?
   Also I can't apply that part of the PR with Eclipse but as it is a unit test I still can test.

2. In multilingual, the Homes can be published up and down.
   Not sure this is not a good idea.
   I explain: although the flag would not display in the switcher on frontend, if the default site language is the one which Home is set with a Publish-Down in the past vs now, the page will display without a menu item and once switched to another language, we can only get back to the published down language by manually adding the url language code for that language.

I know this is already the case if we unpublish such a multilingual Home. But the multilingual status Module at least displays the error.
This is not the case when publish_down and publish_up are used.

We could modify the multilingual status Module but I guess we can simply to totally forbid any home to be saved with publish_up or down > 0.
Something like:

+			if ((int) $this->home)
+			{
+				// Check if publish_down or publish-up are set
+				if ((int) $this->publish_down > 0 || (int) $this->publish_up > 0)
+				{
+
+					$this->setError(JText::_('JLIB_DATABASE_ERROR_MENU_UNPUBLISH_DEFAULT_HOME'));
+
+					return false;
+				}
+			}

This means at least adding a new string
what we have for the the error displayed now is
JLIB_DATABASE_ERROR_MENU_UNPUBLISH_DEFAULT_HOME="Can't unpublish default home."
replace by something like.
JLIB_DATABASE_ERROR_MENU_UNPUBLISH_HOME="Can't set 'Start Publishing' or 'Finish Publishing' for a Home page."

As this would give more infos.

This issue also shows that we do have a kind of bug in multilingual:

We should NEVER be able to display in frontend, even by manually modifying the url, any content language which Home is unpublished. In that case, the default site language could be replaced by one which has a published home, maybe by the first available one in id order.
Let's try to figure how to solve that one.

home (language or not ) cannot be unpublished in the past or in the future

@alikon
I am now looking for a better solution for the homes. Give me some time. :)

@alikon
This is what I propose

1. Modify item.xml to use showon

              <field
			name="publish_up"
			type="calendar"
			label="JGLOBAL_FIELD_PUBLISH_UP_LABEL"
			description="JGLOBAL_FIELD_PUBLISH_UP_DESC"
			id="publish_up"
			class="inputbox"
			translateformat="true"
			showtime="true"
			size="22"
			filter="user_utc"
			showon="home:0"
		/>

		<field
			name="publish_down"
			type="calendar"
			label="JGLOBAL_FIELD_PUBLISH_DOWN_LABEL"
			description="JGLOBAL_FIELD_PUBLISH_DOWN_DESC"
			id="publish_down"
			class="inputbox"
			translateformat="true"
			showtime="true"
			size="22"
			filter="user_utc"
			showon="home:0"
		/>

Then modify menu.php to set publishdown and publishup to null

			if ((int) $this->home)
			{	
				$this->publish_up     = $this->_db->getNullDate();
				$this->publish_down = $this->_db->getNullDate();
			}

Result:
When editing an existing Home menu item, the publish down and up will not even display.
When creating a home menu item and then, before setting the item to home, a date is entered in the publish-up or down, the value will be set to Null.
In any case the value is set to Null when saving.

I think it is more simple this way and it avoids adding an error.

agree,
thanks @infograf768

I have tested this item ✅ successfully on 9f57098

RTC after two successful tests.

@mbabker
Please decide the milestone.

Setting back to pending as we need new tests.

Some issues here
Time Zone set to UTC in global config.
User set to use default.

Start publishing = today
Finishing publishing = tomorrow.

Dates displayed in the fields is French time (CEST) = UTC+2

The result is that the menu item does not display although it should.

After clearing the publish up and down, I do get correctly

hmm, it looks like the issue is unrelated to this PR.
@dgt41 @mbabker
Please look at this. To test, use UTC in global config, clear the start publishing date of an article, save, then click on today.
The article is set Published but pending

@infograf768 cant find it now but i am pretty sure we have an open issue about this exact thing

As I can't find it either, I create a new issue.

See #17770

Apart from that issue, the PR looks good.

I have tested this item ✅ successfully on 9f57098

As the issue is a general issue in core (2.5.28 does not have this problem), this PR is OK for me.

I have tested this item ✅ successfully on 9f57098

RTC after two successful tests.

hope no need to retest, only cosmetic change

@alikon it's an easy Test :-)

URL menu item type does not have this option. Intentional?

URL menu item type does not have this option. Intentional?

Looks like it concerns all System Links.

Found out why.
For these menutypes, we do not display the Home field.
As displaying the publish-up and publish-down fields depends on the setting of the Home (Default Page) to No, they will not display.
Code to not display the Home field for system links is in /administrator/components/com_menus/views/item/tmpl/edit.php line 150-153.

				if ($this->item->type != 'component')
				{
					$this->fields = array_diff($this->fields, array('home'));
				}

Good catch @Quy, it wasn't intentional
@infograf768 can you do the fix ?

On 31 Aug 2017 9:58 am, "infograf768" notifications@github.com wrote:

Found out why.
For these menutypes, we do not display the Home field.
As displaying the publish-up and publish-down fields depends on the
setting of the Home (Default Page) to Yes, they will not display.
Code to not display the Home field for system links is in
/administrator/components/com_menus/views/item/tmpl/edit.php line 150-153.

  		if ($this->item->type != 'component')
  		{
  			$this->fields = array_diff($this->fields, array('home'));
  		}

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17709 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALFsbGXIiBZh3p3VCtLpLcb_1e5LWeaks5sdme8gaJpZM4PCRNE
.

The fix should be to add in /administrator/components/com_menus/views/item/view.html.php line 79.

Add

		// System Links have no home. Force display of publish_up and publish_down.
		if ($this->item->type == 'alias' || $this->item->type == 'url'
			|| $this->item->type == 'separator' || $this->item->type == 'heading')
		{
			$this->form->setFieldAttribute('publish_up', 'showon', '');
			$this->form->setFieldAttribute('publish_down', 'showon', '');
		}

Can be added in fact in edit.php.

@alikon
Committed directly.

@Quy
Can you test?

Thanks @infograf768

On 31 Aug 2017 10:25 am, "infograf768" notifications@github.com wrote:

@alikon https://github.com/alikon
Committed directly.

@Quy https://github.com/quy
Can you test?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#17709 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALFsUWOZRGwbcdLwnPzw0esyM7eg2Ttks5sdm4OgaJpZM4PCRNE
.

@mbabker
Can we get this improvement in 3.8.0 ?

I have tested this item ✅ successfully on 9f57098

I've tested unpublished menu such as "404 not Found" with date in past/future. Working as before.

I see there hasn't been any activity here. Was wondering what the status is and when we can expect this feature?

@TLWebdesign after the needed 2 successfully Tests its a Decision of Release Lead (3.8: @mbabker)

This needs to be tested to see what happens if a menu type of single article (for example) is unpublished and the results of a search tries to load that article

good point
i've made some test with com_search only,
and the article are searched & loaded indipendently if the article is an item of a menu and that menu is not showed cause of publish/unpubish datetime, should be the expected behaviour imo
but i agree more tests are always better

p.s
com_finder should behave the same

yeah thats what i thought. i think users will expect it not to be found if the menu item is unpublished. otherwise they have to unpublish the article and the menu item but they will think that the menu is enough - i know i would have done

as it is a new feature should be enough to details its (logical imho) beahviuor in the documentation ?

I see some things that should be described:

• if we use cache then menu will be unpublished (in the worst case after publish_down + cache timeout)
• for the date after publication (publish_down), the menu item will not be visible in the module menu but links added manual in other places (article) will be changed (after cache timeout), ex:
  • from /category-menu/article-menu-item-with-publish-down
  • to /category-menu/123-article
  • or to /component/content/1-category/123-article

@alikon can you please fix the merge conflicts?

Is this still in the making?

Is this still in the making?
moved on 3.10 branch let's see ?

I think this is such a useful pull request, if you need someone to test it please tag me and I will test it for you @alikon . Let me know whenever you are ready.

This would be very usefull indeed. I’m no help besides testing but jeep up the good work.

@HLeithner This Pull Request have Status "Ready for Commit" and conflicting Files, so Status removed.

But now the 3.10-Milestone is gone; in Issue Tracker Label "RTC" is also shown.

@alikon Nicola can you please fix conflicts?

Conflicting files

installation/sql/mysql/sample_brochure.sql
installation/sql/mysql/sample_learn.sql
installation/sql/mysql/sample_testing.sql
tests/unit/stubs/database/jos_menu.csv

@jeckodevelopment can you reassign 3.10-Milestone?

generally milestone is assigned once in RTC status

i'll be happy to fix conflicts, just want to know, where new feature like this should be supposed to land ?

• 3.9.x
• 3.10.x
• 4.x

p.s
it's surfing the joomla sea from 3.8.0

@HLeithner as RL should be able to help with that decision ?

@alikon please rebase on 4.0, I watched @wilsonge merging 3.9 commits in to 4.0 branch....

done #24416