Skip to content

Fixes Tests #125

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Fixes Tests #125

wants to merge 15 commits into from

Conversation

ismaail
Copy link
Contributor

@ismaail ismaail commented Feb 4, 2025

I managed to fix all failing tests (see Github Actions)

but I had to skip 2 tests and disable webpage in routes as it need to be done in a different way.

@oitcode
Copy link
Owner

oitcode commented Feb 6, 2025

@ismaail Thanks for contributing once again.

Please give me some time to review the changes. Will update you soon.

@ismaail
Copy link
Contributor Author

ismaail commented Feb 6, 2025

@oitcode no worries, take your time.

@ismaail ismaail force-pushed the tests branch 5 times, most recently from 6bf65fd to 4137768 Compare February 11, 2025 15:19
@ismaail ismaail force-pushed the tests branch 2 times, most recently from 646e3a4 to fcded3e Compare February 13, 2025 20:05
@oitcode
Copy link
Owner

oitcode commented Feb 16, 2025

@ismaail Hey, sorry for late response, as I was busy with other schedule.

All the changes look fine, however it seems the webpage routes in the routes/web.php file have been changed. WIthout those routes webpages added from the CMS module will not work correctly. So how to make those webpages work with those changes in routes/web.php file. I got confused. Any suggestions?

@ismaail
Copy link
Contributor Author

ismaail commented Feb 16, 2025

@oitcode instead of creating a route for every webpage in routes/web.php,
my plan is to change this route to something like

Route::get('webpage/{permalink}', 'WebsiteController@webpage')->name('website-webpage');

and handle finding the webpage by permalink in the WebsiteController@webpage,
this way the application don't have to create the webpages routes in routes/web.php every time a webpage is added or deleted.

@ismaail ismaail force-pushed the tests branch 2 times, most recently from 653c8dd to 42108b3 Compare February 22, 2025 18:56
@oitcode
Copy link
Owner

oitcode commented Feb 24, 2025

@ismaail

Sorry again for delay in response.

I was thinking of a way to implement the webpages as

Route::get('/{permalink}', 'WebsiteController@webpage')->name('website-webpage');

because

Route::get('webpage/{permalink}', 'WebsiteController@webpage')->name('website-webpage');

would always have a /webpage/ part in the url for every webpage which is not ideal.

But, as of now I have not clearly figured out how to implement this.

Will update you once I am able to think of something. Your thoughts on this matter?

Also sorry as this PR has already been 3 weeks and am not able to merge it yet. Hope you understand. Thanks.

@ismaail
Copy link
Contributor Author

ismaail commented Feb 24, 2025

I don't think I can add anything new, feel free to update this PR or close it.

Thanks.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants