{"action":"created","issue":{"url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/4072","repository_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck","labels_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/4072/labels{/name}","comments_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/4072/comments","events_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/4072/events","html_url":"https://github.com/rubberduck-vba/Rubberduck/pull/4072","id":331309353,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0MDcxOTkx","number":4072,"title":"Refactoring dialog refactor","user":{"login":"bclothier","id":2367644,"node_id":"MDQ6VXNlcjIzNjc2NDQ=","avatar_url":"https://avatars2.githubusercontent.com/u/2367644?v=4","gravatar_id":"","url":"https://api.github.com/users/bclothier","html_url":"https://github.com/bclothier","followers_url":"https://api.github.com/users/bclothier/followers","following_url":"https://api.github.com/users/bclothier/following{/other_user}","gists_url":"https://api.github.com/users/bclothier/gists{/gist_id}","starred_url":"https://api.github.com/users/bclothier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bclothier/subscriptions","organizations_url":"https://api.github.com/users/bclothier/orgs","repos_url":"https://api.github.com/users/bclothier/repos","events_url":"https://api.github.com/users/bclothier/events{/privacy}","received_events_url":"https://api.github.com/users/bclothier/received_events","type":"User","site_admin":false},"labels":[{"id":765543834,"node_id":"MDU6TGFiZWw3NjU1NDM4MzQ=","url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/labels/PR-Status:%20WIP","name":"PR-Status: WIP","color":"584daf","default":false}],"state":"open","locked":false,"assignee":{"login":"Vogel612","id":7288995,"node_id":"MDQ6VXNlcjcyODg5OTU=","avatar_url":"https://avatars3.githubusercontent.com/u/7288995?v=4","gravatar_id":"","url":"https://api.github.com/users/Vogel612","html_url":"https://github.com/Vogel612","followers_url":"https://api.github.com/users/Vogel612/followers","following_url":"https://api.github.com/users/Vogel612/following{/other_user}","gists_url":"https://api.github.com/users/Vogel612/gists{/gist_id}","starred_url":"https://api.github.com/users/Vogel612/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Vogel612/subscriptions","organizations_url":"https://api.github.com/users/Vogel612/orgs","repos_url":"https://api.github.com/users/Vogel612/repos","events_url":"https://api.github.com/users/Vogel612/events{/privacy}","received_events_url":"https://api.github.com/users/Vogel612/received_events","type":"User","site_admin":false},"assignees":[{"login":"Vogel612","id":7288995,"node_id":"MDQ6VXNlcjcyODg5OTU=","avatar_url":"https://avatars3.githubusercontent.com/u/7288995?v=4","gravatar_id":"","url":"https://api.github.com/users/Vogel612","html_url":"https://github.com/Vogel612","followers_url":"https://api.github.com/users/Vogel612/followers","following_url":"https://api.github.com/users/Vogel612/following{/other_user}","gists_url":"https://api.github.com/users/Vogel612/gists{/gist_id}","starred_url":"https://api.github.com/users/Vogel612/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Vogel612/subscriptions","organizations_url":"https://api.github.com/users/Vogel612/orgs","repos_url":"https://api.github.com/users/Vogel612/repos","events_url":"https://api.github.com/users/Vogel612/events{/privacy}","received_events_url":"https://api.github.com/users/Vogel612/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/milestones/30","html_url":"https://github.com/rubberduck-vba/Rubberduck/milestone/30","labels_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/milestones/30/labels","id":3881088,"node_id":"MDk6TWlsZXN0b25lMzg4MTA4OA==","number":30,"title":"2.4.0","description":"Reference Explorer and whatever else we get done by then will make a great christmas / new year's present to the VBA / VB6 community :)","creator":{"login":"Vogel612","id":7288995,"node_id":"MDQ6VXNlcjcyODg5OTU=","avatar_url":"https://avatars3.githubusercontent.com/u/7288995?v=4","gravatar_id":"","url":"https://api.github.com/users/Vogel612","html_url":"https://github.com/Vogel612","followers_url":"https://api.github.com/users/Vogel612/followers","following_url":"https://api.github.com/users/Vogel612/following{/other_user}","gists_url":"https://api.github.com/users/Vogel612/gists{/gist_id}","starred_url":"https://api.github.com/users/Vogel612/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Vogel612/subscriptions","organizations_url":"https://api.github.com/users/Vogel612/orgs","repos_url":"https://api.github.com/users/Vogel612/repos","events_url":"https://api.github.com/users/Vogel612/events{/privacy}","received_events_url":"https://api.github.com/users/Vogel612/received_events","type":"User","site_admin":false},"open_issues":10,"closed_issues":10,"state":"open","created_at":"2018-12-07T22:03:38Z","updated_at":"2018-12-20T15:27:46Z","due_on":"2018-12-30T08:00:00Z","closed_at":null},"comments":3,"created_at":"2018-06-11T18:57:34Z","updated_at":"2018-12-29T01:40:39Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/pulls/4072","html_url":"https://github.com/rubberduck-vba/Rubberduck/pull/4072","diff_url":"https://github.com/rubberduck-vba/Rubberduck/pull/4072.diff","patch_url":"https://github.com/rubberduck-vba/Rubberduck/pull/4072.patch"},"body":"This PR Is WIP, pending unit test revisions. However, I want to put up for review by others while I work on the unit tests.\r\n\r\nThe refactor of the refactoring dialogs aims to do the following:\r\n\r\n1) abstract the MVP layer out of the refactoring's concern. In the end, a refactoring class only care about the model and it should be providing a model, and then getting back a model (which may be modified or not) to then proceed with the refactoring.\r\n\r\n2) Move all the logic that don't have to do with UI (e.g. nulling the model) out of presenter factories and into the refactoring, typically set up as `InitializeModel` private method.\r\n\r\n3) Make all refactoring dialogs & presenters use a common generic base to maximize the code reuse, and ensure that all concerns are handled within the view and viewmodel of the MVVM layer.\r\nAll refactoring presenters now derive from `RefactoringPresenterBase<TModel, TDialog, TView, TViewModel>` base. All refactoring dialogs, `RefactoringDialogBase<TModel, TView, TViewModel>`. Refactoring viewmodels for those w/ dialogs derive from `RefactoringViewModel<TModel>`. \r\n\r\nNote that the effort is put in to abstract all the generics away. From the refactoring's POV, they know nothing about the generic types put in because a concrete class is provided. For example, `ExtractInterfaceRefactoring` only needs to ask for `IExtractInterfacePresenter`, which CW will provide the concrete `ExtractInterfacePresenter` class, with all the type arguments provided for.\r\n\r\n4) Make use of CW automagic factory to help with creation of presenter and dialog both to reinforce the separation of concerns between the MVP layer (which should be generic) and the MVVM layer (which usually will be specific to each refactoring).\r\n\r\n5) Remove the `System.Windows.Forms` out of the `Rubberduck.Refactoring` project. I am in the paradoxical position where I need to define the UI interfaces in the project but provide the concretes in the `Rubberduck.Core\\UI\\Refactoring`. \r\n\r\n6) Move the OK and Cancel command for the VM into the base class.\r\n\r\n7) Provide an abstraction for the confirm message in order to decouple the messagebox dependency from the refactoring class, so that it is a strictly a UI dependency rather than a refactoring dependnecy.\r\n\r\nAdditionally fix the splash icon to use the ducky.ico (it wasn't and used the default icon in the taskbar while it's running).\r\n\r\nTODO:\r\n\r\n1) See if we can remove the need to set the captions\r\n2) and likewise the size. I tried to get the dialogs derive from the viewmodel's size but I've found that WPF has a weird notion of what is a \"preferred\" size. :\\ \r\n\r\nResolving the #1 and #2 would remove the need to derive from the `RefactoringDialogBase` and thus can allow direct use of the base class instead for some of the refactoring classes.\r\n\r\n3) Fix up the unit tests. \r\n\r\nAny comments/feedbacks are welcome. "},"comment":{"url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/comments/450453438","html_url":"https://github.com/rubberduck-vba/Rubberduck/pull/4072#issuecomment-450453438","issue_url":"https://api.github.com/repos/rubberduck-vba/Rubberduck/issues/4072","id":450453438,"node_id":"MDEyOklzc3VlQ29tbWVudDQ1MDQ1MzQzOA==","user":{"login":"bclothier","id":2367644,"node_id":"MDQ6VXNlcjIzNjc2NDQ=","avatar_url":"https://avatars2.githubusercontent.com/u/2367644?v=4","gravatar_id":"","url":"https://api.github.com/users/bclothier","html_url":"https://github.com/bclothier","followers_url":"https://api.github.com/users/bclothier/followers","following_url":"https://api.github.com/users/bclothier/following{/other_user}","gists_url":"https://api.github.com/users/bclothier/gists{/gist_id}","starred_url":"https://api.github.com/users/bclothier/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bclothier/subscriptions","organizations_url":"https://api.github.com/users/bclothier/orgs","repos_url":"https://api.github.com/users/bclothier/repos","events_url":"https://api.github.com/users/bclothier/events{/privacy}","received_events_url":"https://api.github.com/users/bclothier/received_events","type":"User","site_admin":false},"created_at":"2018-12-29T01:40:39Z","updated_at":"2018-12-29T01:40:39Z","author_association":"CONTRIBUTOR","body":"With the commits, the PR is now in a workable state that can be merged as-is. There are a number of todos:\r\n\r\n1) Further clean up of the unit tests; some are marked with FIXME as they are now obsolete (e.g. originally testing that smart presenter is working) and needs to be revised to test the equivalent concern that may have been further cleaned up.\r\n\r\n2) Further refinement and DRYing the code for the dialogs. As an example, we have to manually specify the size for dialogs. Another example is cutting down on the repetition of the interface type specifications in the declaration. I think we can do more to trim the code.\r\n\r\n3) Make the design more consistent. As an example, before the refactoring, we had concerns all over the place among the smart presenter, the model, the viewmodel. The refactoring now has narrowed the concern down to only the model so that refactoring class only need to create a model, provide it to the presenter and get the potentially modified model back. However, we can do more to make it even more uniform -- the viewmodel should ensure that it dispatch changes to the model directly as opposed to creating a new model. \r\n\r\nOn the other hand, the PR is already large and very old so I'm feeling inclined to get it merged and defer those issues into a new separate PR that's more focused. However, I'd rather that it was carefully scrutinized  and reviewed before we merge this sprawling PR.\r\n\r\nThoughts, @rubberduck-vba/core ?"}}
{"actor":{"display_login":"bclothier"}}