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

#628 progress default page changed #1590

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ddm14159
Copy link
Contributor

@ddm14159 ddm14159 commented Sep 1, 2023

#628 По умолчанию открываем первый раздел, в котором есть непрочитанные главы. Если прочитано все, то показываем начало
https://ddm-hexlet-sicp.onrender.com/ru/my

@sgmdlt
Copy link
Contributor

sgmdlt commented Sep 4, 2023

@fey

Copy link
Collaborator

@fey fey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я бы предложил переписать код. Смотрите, у нас есть сервисы https://github.com/Hexlet/hexlet-sicp/tree/main/app/Services
и там можно создать chapterService и добавить всю логику по работе с главами.
Т.е. если пользователь отмечает прочитанной главы - то проверяем, все ли они относятся к mainChapter.
Соответственно, нам бы оставалось просто брать главные главы книги и главные главы пользователя, и строить запрос, чтобы получать первую главу, которая еще не отмечена прочитанной.

@@ -21,7 +21,19 @@ public function __invoke(): View
$user->load('readChapters', 'exerciseMembers');

$chapters = Chapter::with('children', 'exercises')->get();
$mainChapters = $chapters->where('parent_id', null);
$mainChapters = $chapters->where('parent_id', null)->sortBy('id');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

давайте добавить scope для получения главных глав.
По сортировке- надежнее использовать сортировку по path. Тогда это будет по смыслу ближе к нашей предметной области. ведь у наших глав в книге нет айдишников, есть ее номера.


public function haveReadMainChapter(Chapter $chapter): bool
{
$subChapters = ChapterHelper::getChapterDescendants($chapter);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

На мой взгляд неправильно прокидывать в модель хелпер.
Хелперы служат для удобного доступа к некоторым данным, чтобы не дулировать код. Чаще всего они используются во вьюхах, контроллерах. Но на самом деле некоторые из них уже содержат код, который можно сказать принадлежит нашей предметной области и пора это добро закинуть в в какой-то сервис что ли.

@ola-9
Copy link

ola-9 commented Sep 11, 2023

@ddm14159 скажите, пожалуйста, нужна ли какая-нибудь помощь, как продвигается работа?

@fey fey marked this pull request as draft September 11, 2023 10:13
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.

None yet

4 participants