Skip to content

fix package#69

Open
wolfrednicolas wants to merge 14 commits into
mainfrom
wolfrednicolas/feature/fix-packages-to-work-in-nova
Open

fix package#69
wolfrednicolas wants to merge 14 commits into
mainfrom
wolfrednicolas/feature/fix-packages-to-work-in-nova

Conversation

@wolfrednicolas
Copy link
Copy Markdown
Contributor

  • fix policies and unit test in the scheduler package

Comment thread src/Nova/EscaperoomSlot.php Outdated
})->exceptOnForms(),
DateTime::make('Start At')->required(),
nova('rate') ? BelongsTo::make('Rate', 'rate', nova('rate'))->hideWhenCreating()->nullable() : null,
//TODO is missing the morph relation with schedule_id and schedule_type and the models
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@drewroberts what will be the models for this morph relation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pdbreen Could you start taking a look at this package and give us some direction on changes you want to make and how to get it structured?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@wolfrednicolas here's an example of how to setup morph relationships in Nova without knowing which packages will be installed: https://github.com/tipoff/notes/blob/f20b9ed1dbcd98d49e3d479f4c5bd830e6787f19/src/Nova/Note.php#L48

            MorphTo::make('Noteable')->types(array_filter([
                nova('user'),
                nova('contact'),
                nova('customer'),
                nova('order'),
                nova('booking'),
                nova('game'),
                nova('block'),
                nova('slot'),
            ])),

@drewroberts - it looks like tipoff/addresses needs an issue created to fix the Nova addressable morph to use this technique. The addressable type list should include cart, order, voucher, and probably a bunch of other things, but it needs to be done as above so the packages remain optional.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@joshtorres @phuclh Could you create an issue for this and get this implemented in the addresses package?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you @pdbreen I replicate the same structure here. 👍

@wolfrednicolas wolfrednicolas added the question Further information is requested label Apr 22, 2021
@drewroberts drewroberts requested a review from pdbreen April 23, 2021 12:05
@drewroberts drewroberts requested a review from kylebarney April 24, 2021 17:42
@drewroberts
Copy link
Copy Markdown
Member

@kylebarney Could you work on this one today to get it ready to merge?

@wolfrednicolas wolfrednicolas removed the question Further information is requested label Apr 24, 2021
Comment on lines +85 to +92
nova('user'),
nova('contact'),
nova('customer'),
nova('order'),
nova('booking'),
nova('game'),
nova('block'),
nova('slot'),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't look like the proper list for this morph. The comment was only showing proper technique, not the desired set of objects.

Copy link
Copy Markdown
Contributor Author

@wolfrednicolas wolfrednicolas Apr 24, 2021

Choose a reason for hiding this comment

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

@drewroberts could you give me the nova resources related to schedule morph relation?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this one resolved?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hey @joshtorres is missing the list of objects related to that morph relation, different than that is ready to merge

Comment thread src/Nova/RecurringSchedule.php Outdated
Comment thread src/Nova/RecurringSchedule.php Outdated
Copy link
Copy Markdown
Member

@joshtorres joshtorres left a comment

Choose a reason for hiding this comment

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

Thank you!

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.

4 participants