Handle empty module on the course structure saving

[merkushin]

Resolves #7625

The problem described in the issue happens when, while following the tour, the user doesn't set the module title.

The course structure can't be saved in this case, and it causes the issue that the expected “Edit Lesson” link doesn't appear in the toolbar.

Proposed Changes

• Adjust the text in the tour to instruct the user to set a title for the module.
• Set default titles for modules on saving.

Testing Instructions

1. On a new website, create a course, and follow the next steps before saving/publishing the course.
2. Take the tour and on the third step (“Adding a module”) make sure we mention the need to add the title for the module: “Then click the Module option and give it a name.”
3. Add a few modules without setting titles for them.
4. Save the draft.
5. Make sure each module without a name got a title like “Module N” where N is an ordinal number.

Pre-Merge Checklist

• PR title and description contain sufficient detail and accurately describe the changes
• Acceptance criteria is met
• Decisions are publicly documented
• Adheres to coding standards (PHP, JavaScript, CSS, HTML)
• All strings are translatable (without concatenation, handles plurals)
• Follows our naming conventions (P6rkRX-4oA-p2)
• Hooks (p6rkRX-1uS-p2) and functions are documented
• New UIs are responsive and use a mobile-first approach
• New UIs match the designs
• Different user privileges (admin, teacher, subscriber) are tested as appropriate
• Legacy courses (course without blocks) are tested
• Code is tested on the minimum supported PHP and WordPress versions
• User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
• “Needs Documentation” label is added if this change requires updates to documentation
• Known issues are created as new GitHub issues

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[github-actions]

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit 8eab758 and trunk. Please review and confirm the following are correct before merging.

Script Handle	Added Dependencies	Removed Dependencies	Total Size	Size Diff
admin/tour/course-tour/index.js			35.9 kB	+8 B ( +0.03% 🔼 )

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

[github-actions]

Test the previous changes of this PR with WordPress Playground.

[m1r0]

Looks good and works as described. 👍

As a side note, I've noticed that on the Mac, the tour hides the module so it's hard to see. This could be addressed in another PR.

[Imran92]

LGTM, just added two small comments.

[Imran92]

Nitpick: Just an edge case as we are trying to work against user-made scenarios - can we trim the title and then check if it's empty? Because otherwise, it doesn't assign a name to the module if the title is just one or more spaces. Does it make sense?

[Imran92]

Has this check become redundant now?

sensei/includes/class-sensei-course-structure.php

Lines 897 to 902 in 53dbffa

	if ( 'module' === $raw_item['type'] ) {
	return new WP_Error(
	'sensei_course_structure_modules_missing_title',
	__( 'Please ensure all modules have a name before saving.', 'sensei-lms' )
	);
	}

[renatho]

Hey! 👋
Just a suggestion. I thought we would implement it as a solution applied only to the tour (populating the module names if needed as part of one of the steps). Wouldn't it be better?

[Imran92]

Thanks for raising it @renatho !

Hey! 👋
Just a suggestion. I thought we would implement it as a solution applied only to the tour (populating the module names if needed as part of one of the steps). Wouldn't it be better?

It was exactly my thought and I agree it would be better (That's why my suggestion here p1722358285119989/1722355438.888029-slack-C02NWDZBL0H was to add the names before "triggering" save, we trigger a save during the tour). I thought about commenting the same here, but later thought as this is already implemented, it'd make us re-do the whole thing, that's why I approved it, thinking about the cost vs. benefit. But I would've preferred doing it only for the tour to keep the flow of the tour smooth and errorless, and enforce user to explicitly give modules a Title as before and not saving otherwise in rest of the cases.

[merkushin]

@renatho @Imran92

I thought we would implement it as a solution applied only to the tour (populating the module names if needed as part of one of the steps). Wouldn't it be better?

That's why my suggestion here p1722358285119989/1722355438.888029-slack-C02NWDZBL0H was to add the names before "triggering" save, we trigger a save during the tour.

Yes, initially tried to do that on the frontend, but didn't find a proper way to update titles, and switched to a more straightforward solution.

Also, speaking about the frontend, I noticed that we only show the error message at the top of the screen, but don't even highlight the fields that require user's action (and I think because of placeholders it might not be obvious for some users). So I was thinking that setting the default title on the backend is a quick solution to this problem as well as it forgives some user mistakes, and we don't need to work on UX and design for that.

I'm going to look maybe for a quick dirty solution for the approach Imran suggested.

[merkushin]

@renatho @Imran92 here is the new PR with setting titles only in the tour: #7652