Issue/44 friendlier zone lock#74
Conversation
Zoninator#44 Updated the redirect url to use when the zone lock expires.
Zoninator Automattic#44 Changed the `error-zone-lock-max` message to reflect the updated redirect url.
Zoninator Automattic#44 Added an admin notice when a user's zone lock expires and they get redirected to the top level zones page.
sboisvert
left a comment
sboisvert
left a comment
There was a problem hiding this comment.
Looks good. One small question
zoninator.php
Outdated
There was a problem hiding this comment.
What happens here if they pass a zone_id that doesn't exist?
There was a problem hiding this comment.
@sboisvert Ah, yeah, I'll need to only display the notice if $zone is not false.
Do you think there should be a different notice if $_GET['zone_lock'] isn't a valid Zone ID or ignore it? I think that should only be able to happen if:
- The zone is deleted as the user who was editing the zone is being redirected
- Someone purposefully puts an invalid Zone ID in the
zone_lockurl param
There was a problem hiding this comment.
Nod I agree on when it should only happen. I suspect that this will throw a PHP Warning if that's the case since we're using the return of $this->get_zone() without checking if it's actually what we expect as per https://vip.wordpress.com/documentation/code-review-what-we-look-for/#not-checking-return-values :)
There was a problem hiding this comment.
Correct. It does throw a warning there if you pass an invalid Zone ID into $_GET['zone_lock'].
Thoughts on adding a different notice if the Zone ID is invalid?
Zoninator Automattic#44 We needed to check and make sure that `$zone` is not `false` coming back from `$this->get_zone()` otherwise a PHP Warning is thrown.
Zoninator Automattic#44 Fixed a whitespace issue in new code.
3 participants
Zoninator #44
error-zone-lock-maxmessage to reflect the updated redirect url.