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.

Sign up for GitHub

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

Jump to bottom

Allow data provider to throw ItemNotFoundException #2884

Closed
wants to merge 1 commit into from
Closed

Allow data provider to throw ItemNotFoundException #2884

wants to merge 1 commit into from

Conversation

teohhanhui
Copy link
Contributor

@teohhanhui teohhanhui commented Jun 26, 2019 •
edited

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

Converted to NotFoundHttpException in ReadListener

TODO:

  • Add tests

@teohhanhui
Copy link
Contributor Author

teohhanhui commented Jun 26, 2019 •
edited

phpstan errors are not related (needs to be fixed in 2.4).

@teohhanhui teohhanhui force-pushed the feature/allow-data-provider-throw-item-not-found-exception branch from 215942c to c38265f Compare June 27, 2019 11:01
@teohhanhui teohhanhui mentioned this pull request Jun 27, 2019
Merged
@teohhanhui teohhanhui force-pushed the feature/allow-data-provider-throw-item-not-found-exception branch from c38265f to a1d0884 Compare June 27, 2019 14:17
@teohhanhui
Allow data provider to throw ItemNotFoundException
0de2987 
Converted to NotFoundHttpException in ReadListener
@teohhanhui teohhanhui force-pushed the feature/allow-data-provider-throw-item-not-found-exception branch from a1d0884 to 0de2987 Compare July 5, 2019 18:52
@teohhanhui teohhanhui mentioned this pull request Jul 8, 2019
Merged
@teohhanhui
Copy link
Contributor Author

I'll add tests.

soyuka
soyuka requested changes Jul 16, 2019
View reviewed changes
Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

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

need tests :D

@teohhanhui
Copy link
Contributor Author

Yes, I'll add them when I have time to work on this, since #2934 has been merged.

Base automatically changed from master to main January 23, 2021 21:59
@soyuka soyuka closed this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@soyuka soyuka soyuka requested changes

@dunglas dunglas dunglas approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@teohhanhui @dunglas @soyuka