{"action":"created","issue":{"url":"https://api.github.com/repos/woocommerce/woocommerce/issues/12619","repository_url":"https://api.github.com/repos/woocommerce/woocommerce","labels_url":"https://api.github.com/repos/woocommerce/woocommerce/issues/12619/labels{/name}","comments_url":"https://api.github.com/repos/woocommerce/woocommerce/issues/12619/comments","events_url":"https://api.github.com/repos/woocommerce/woocommerce/issues/12619/events","html_url":"https://github.com/woocommerce/woocommerce/issues/12619","id":196023521,"number":12619,"title":"Compatibility issue - Properties like $id and $variation_id became \"protected\"","user":{"login":"daigo75","id":292434,"avatar_url":"https://avatars.githubusercontent.com/u/292434?v=3","gravatar_id":"","url":"https://api.github.com/users/daigo75","html_url":"https://github.com/daigo75","followers_url":"https://api.github.com/users/daigo75/followers","following_url":"https://api.github.com/users/daigo75/following{/other_user}","gists_url":"https://api.github.com/users/daigo75/gists{/gist_id}","starred_url":"https://api.github.com/users/daigo75/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/daigo75/subscriptions","organizations_url":"https://api.github.com/users/daigo75/orgs","repos_url":"https://api.github.com/users/daigo75/repos","events_url":"https://api.github.com/users/daigo75/events{/privacy}","received_events_url":"https://api.github.com/users/daigo75/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2016-12-16T10:09:51Z","updated_at":"2016-12-16T12:43:36Z","closed_at":"2016-12-16T12:43:36Z","body":"**Issue introduced in WooCommerce 2.7**\r\nThere is a potentially serious issue with how the product classes are structured in WC 2.7. Properties like `$product->id` and `$product->variation_id` have historically been `public`. It's common practice, for the visibility of object properties, to be \"one way only\": properties are introduced as `protected`, then they may become `public`, but don't go back to `protected`. This is a reasonable assumption, which holds true in (almost) all project I've ever worked on.\r\n\r\nIn WC 2.7, properties like `$id` and `$variation_id` became `protected`, and that will cause issues with plugins that expect them to be accessible (i.e. quite a large amount of them).\r\nI do understand that PHP doesn't provide a mechanism to have read-only public properties, and that the new abstraction layer might need that type of protection, but, in my opinion, properties like `$product->id`, `$order->id` and so on should be at least \"soft deprecated\" and left public for the moment. This will make it easier to switch to the new methods `WC_Product::get_id()` and `WC_Order::get_id()`, by keeping backward compatibility with WC 2.6 and without having to litter the code with \"if WC version\" (`WC_Order::get_id()` doesn't exist in WC 2.6 and earlier, only the product classes had that method)."},"comment":{"url":"https://api.github.com/repos/woocommerce/woocommerce/issues/comments/267586611","html_url":"https://github.com/woocommerce/woocommerce/issues/12619#issuecomment-267586611","issue_url":"https://api.github.com/repos/woocommerce/woocommerce/issues/12619","id":267586611,"user":{"login":"mikejolley","id":90977,"avatar_url":"https://avatars.githubusercontent.com/u/90977?v=3","gravatar_id":"","url":"https://api.github.com/users/mikejolley","html_url":"https://github.com/mikejolley","followers_url":"https://api.github.com/users/mikejolley/followers","following_url":"https://api.github.com/users/mikejolley/following{/other_user}","gists_url":"https://api.github.com/users/mikejolley/gists{/gist_id}","starred_url":"https://api.github.com/users/mikejolley/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mikejolley/subscriptions","organizations_url":"https://api.github.com/users/mikejolley/orgs","repos_url":"https://api.github.com/users/mikejolley/repos","events_url":"https://api.github.com/users/mikejolley/events{/privacy}","received_events_url":"https://api.github.com/users/mikejolley/received_events","type":"User","site_admin":false},"created_at":"2016-12-16T12:43:36Z","updated_at":"2016-12-16T12:43:36Z","body":"> Correction: as @mikejolley pointed out, the __get() methods would allow to access the $id property, although they would raise a warning (this is also something that, in my opinion, could be improved, as PHP warnings are too \"alarming\" to the average Joe). The issue will arise if a protected $id is being written (unless there is a __set() method that I haven't spotted).\r\n>\r\n> The question is: would anyone need to alter the $id property? Maybe. I remember that I wrote some code that does that, in the past, which was in my \"to refactor\" list, but there might be some use cases in which a writable $id might be needed.\r\n\r\nThe `set_id()` method is public, so this is allowable and CRUD will know of the change :)\r\n\r\nThere are other benefits to this change btw. I don't know if you noticed, but in legacy we made a real mess of variation ids. ->id was the parent id. With the new `get_id`/`get_parent_id` this is resolved.\r\n\r\nSince we offer getters and setters for IDs I don't think we have a serious issue here. I'll certainly do that post on deprecation notices, and perhaps make a wiki entry too which we can link to from our warnings  - I think that would be useful for non-devs running WP_DEBUG by mistake."}}
{"actor":{"display_login":"mikejolley"}}