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

[Docs] Correct example showing the record passed in as param #7657

Merged

Conversation

x7ryan
Copy link
Contributor

@x7ryan x7ryan commented Aug 11, 2023

When following this example I discovered I could not call $this->record from that context. After a quick check I discovered the $record must be passed in as a parameter into the closure.

NOTE: There are other examples on this docs page of $this->record that I suspect may also need to be updated but I have not tested that so I am not updating them, unless someone can confirm that is also the case there. I am making this a draft until I can get confirmation on this.

@x7ryan x7ryan marked this pull request as draft August 11, 2023 01:53
@what-the-diff
Copy link
Contributor

what-the-diff bot commented Aug 11, 2023

PR Summary

  • Update to the Action Method Callback
    The changes in this PR basically improve the functioning of certain methods within our app. Previously, only one parameter was included in our action method 'callback'. But now, a second parameter known as 'Model $record' has been added. This change allows the method to receive more data to work with, thus increasing its effectiveness.

  • Record Reference Update
    The amendments have also affected the usage of '$this->record' references as they were changed to a simpler format- just '$record'. This makes the code more straightforward and reduces ambiguity.

  • Author Association Change
    The line that associated an author with a given record was modified too. Before it used to be '$this->record->author()->associate($data['authorId'])', but after the change it is now '$record->author()->associate($data['authorId'])'. This streamline ensures that an author can be easily linked with their specific work.

  • Removal of 'required' Method on 'authorId' Option
    Lastly, the '.required()' method that was previously called on the 'authorId' option was omitted. We found this wasn't necessary and was actually causing some inefficiencies in our code, as it required certain data to be unnecessarily checked before being processed. This change simply allows for smoother operation as our application no longer needs to check if 'authorId' is present every time.

@danharrin
Copy link
Member

Hmm I think you're right and we should move all examples to $record. But $this->record does still work in some places.

@danharrin danharrin added the documentation Improvements or additions to documentation label Aug 11, 2023
@danharrin danharrin added this to the v3 milestone Aug 11, 2023
@x7ryan
Copy link
Contributor Author

x7ryan commented Aug 11, 2023

Yeah I wasn't sure. But I can go ahead and just switch all the examples to $record. Personally that's probably how I'd actually do it instead of having to figure out/remember where $this does and does not work.

@danharrin danharrin merged commit 6ecd6ff into filamentphp:3.x Sep 9, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants