Updated: Comment #N
Problem/Motivation
So in porting http://drupal.org/project/pants module, naturally I added views integration for my "Recent pants" block, because duh! ;) I spent a good 20 minutes clicking together something totally awesome in the UI, and now I want to move it into my module as a default view.
In the past this would be an "Export" operation off the main page dropbutton, but of course that's not there because CMI exists. However, the workflow for getting this out of CMI and into my module is not totally trivial. I have to do digging around in my sites/default/files/config_jhGjasasduad directory.
I can see this being a "won't fix, you're a developer, so suck it up" which would be fair enough, but it's a bit of a DX loss, so figured I'd start a discussion about it.
Proposed resolution
Introduce a new controller which provides a generic export for, which is then used by views and is accessible from the config entity listing.
User interface changes
Added a export configuration/link.
Comment | File | Size | Author |
---|---|---|---|
#57 | config-1898794-57.patch | 8.08 KB | tim.plunkett |
#55 | interdiff.txt | 655 bytes | tim.plunkett |
#55 | config_entity-export-1898794-55-A.patch | 23.46 KB | tim.plunkett |
#55 | config_entity-export-1898794-55-B.patch | 16.41 KB | tim.plunkett |
#55 | config_entity-export-1898794-55-C.patch | 7.95 KB | tim.plunkett |
Comments
Comment #1
dawehnerPersonally I really think this is quite important if we want to have at least some way of issue-queue-maintainability.
One question is whether we want to expose the actual path to the config file in the UI, or just provide an export. Are there security problems when you have the full path to a config file? It could be problematic when you can access all of them.
Comment #2
tim.plunkettHonestly, having a display exactly like in D7 (a textarea you can click in, select all, copy) would be ideal.
Not everyone will have file system access, and as a module author it'll even make it easier to click together a view, and then copy it to make a mymodule/config/views.view.mymodule_view.yml file out of it.
Comment #3
dawehnerComment #4
moshe weitzman CreditAttribution: moshe weitzman commentedI agree that this would be nice. Even nicer would be a textarea foe importing your config, like Views for D7. I looked around and did not see an issue for the import part.
Setting to CNW because everything changed :)
Comment #5
moshe weitzman CreditAttribution: moshe weitzman commentedRetitle and refile against config entity system
Comment #6
Anonymous (not verified) CreditAttribution: Anonymous commentedoh, this is a nice idea, exporting config thingies should be easy.
don't think i like putting the import a config thingie in the same issue, however.
that's nowhere near as simple, so should be it's own issue.
Comment #7
moshe weitzman CreditAttribution: moshe weitzman commentedNo interest in the export part of this?
Comment #8
tim.plunkettThis was ooollllddddd, no interdiff possible.
I didn't tiptoe around here, I just did it generic from the start.
I also had to fix the views breadcrumbs, the export page's were wrong.
Needs tests.
Comment #9
tim.plunkettOopps.
Comment #11
dawehnerAre you sure we want to have this full information as part of the breadcrumb?
Comment #12
moshe weitzman CreditAttribution: moshe weitzman commentedI think a long unambigous title is fine, personally.
Comment #13
dawehnerComment #14
tim.plunkettRerolling, adding tests
Comment #15
tim.plunkettThe long title was in D7.
Comment #17
tim.plunkettFixed the test, and changed some comments according to beejeebus's feedback.
Comment #18
dawehnerThe diff of this class could have been dramatically reduced.
Yeah snick in views everywhere!
Any reason to not use the Yaml component to produce the expected output?
Missing "\"
Urgs
Comment #19
tim.plunkettI had to add ConfigStorageControllerInterface, and I didn't want to do it wrong :(
Yes, using the Yaml component is much better. I also realized that since we're supposed to be providing UUIDs for default config, we can hardcode that here.
Comment #20
dawehnerThank you!
Let's use 76696577-7364-7572-7061-6c636f726512 instead.
Comment #21
tim.plunkettYou're absolutely right.
Comment #22
dawehnerAdded an issue summary so this is ready to fly now.
Comment #23
tim.plunkettRerolled for the Plugin/Core/Entity move
Comment #24
damiankloip CreditAttribution: damiankloip commentedDo we need to assign this variable?
Can't we just use assertFieldByXpath() here instead?
just wondering, and this will be a follow up if anything, but do you think we should add a setting to entity info or just a controller property for whether the entity should provide an export form. Similar to what we do for status.
Comment #25
alexpottPatch no longer applies
Comment #26
tim.plunkettRerolled, and addressed 24.1
Comment #28
tim.plunkettWhoops, bad reroll on my part. I dropped the one hunk and put the other in the wrong order.
Comment #29
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #30
alexpottComment #31
tim.plunkettBreaking my own patches now (that was just a $this->t() conflict)
Comment #32
webchickSince this is a regression, it seems more task-y than feature-y.
Comment #33
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #34
tim.plunkett#31: config-entity-1898794-31.patch queued for re-testing.
Comment #34.0
tim.plunkettadded issue summary
Comment #35
Dries CreditAttribution: Dries commentedI'd like to see a bit of help text in https://drupal.org/files/export2.png. Without an explanation, this will be rather confusing to most users.
We should also fix the capitalization '(e.g. 'Export View" -> "Export view").
Otherwise looks good!
Comment #36
tim.plunkettI'm honestly unsure of what to say here.
First of all, we don't yet have #1876906: Implement hook_help() for views_ui.module.
Secondly, a page like admin/config/development/export (the closest approximation to this) just says:
I'd like to consider merging the help text into the views_ui_help() issue, and getting this in now.
However, someone on IRC suggested we include the filename, instead of the (useless) name of the entity type.
Here is a screenshot of this in D7:
Here is the new screenshot for D8:
Comment #37
jibranSome minor issues.
entity.manager now.
I think we can add cancel link or back button. Just a suggestion.
Creates
Deletes
Comment #38
tim.plunkett1) Fixed (there are others in core btw)
2) They have a browser back button or the breadcrumbs. We're fixing a regression, D7 didn't have a link.
3+4) That exists in HEAD, this is just moved, but fixed all the same.
Comment #39
dawehnerThere is no action available on the page, so cancel might be even more confusing.
I think show the filename is enough for some context to figure out what is going on.
Comment #40
damiankloip CreditAttribution: damiankloip commentedAgreed, +1. This gets us back to where we were.
Comment #41
jibranThank you for the changes @tim.plunkett. +1 for RTBC.
Comment #42
webchickSorry, but I am with Dries here. That's really awesome that I have an export of my View but... now what? Unlike Views in D7 and below, there's no "Import" to put it into, so this form is a bit of a dead end.
Looking at the patch, this doesn't seem like something we'd want to add in views_help() (or else every single module exposing an "export" key would have to do the same), but rather something we'd want embedded into the form itself.
So I'm thinking maybe a #description on $form['export'] something like:
"Copy and paste this code to your staging configuration directory." (link to wherever docs are that explain what the heck that is.)
Comment #43
moshe weitzman CreditAttribution: moshe weitzman commentedWell, let's add a UI for pasting in the contents of a config file, then. It would be useful for add/edit operations. It would not be used for rename or delete. It would be a simple form which asks for a config name and config body. This would complement our existing form which is for importing a tarball of config files. Drush already does this - the code is pretty simple. I *might* even be able to code it.
In any case, I think we could proceed with this and let core or contrib add the "import single file" form.
Comment #44
tim.plunkett#42 fair enough. This links to https://drupal.org/documentation/administer/config
#43 sounds like an interesting follow-up.
Comment #45
Anonymous (not verified) CreditAttribution: Anonymous commentedyeah, i like the fact that we are trying to raise the 'what do i do with this?' question, but moving this to the staging dir is not an option.
we don't import single files form there, so we need better help text than that. thinking on it.
Comment #46
tim.plunkettSee, that's another reason I didn't want to add help text.
Because of the full-tree-config import thing, this is useless for site building.
It's main usecase was for support requests. The Views queue is driven by people uploading their view so we can spot problems.
Ideally a module developer wanting to provide their own config would be able to find it in their install...
If a single-file import was implemented, this would certainly be insanely useful, and we could document how to use them in conjunction.
So, we have a fixed regression, a patch with unhelpful/misleading UI text, and no suggestions yet. I'm unassigning myself.
Comment #47
dawehnerIt's main usecase was for support requests. The Views queue is driven by people uploading their view so we can spot problems.
I can't tell you how important that was when doing support questions. People can't and also don't want really describe how they had setup their view.
If people click on that link they either know what they really need, or they just got asked to do so.
The title works with #title now, so let's remove the extra file.
Comment #48
dawehnerHey, missed the screenshot.
Comment #49
Anonymous (not verified) CreditAttribution: Anonymous commentedi think this is good to go.
would be great to do what Moshe suggests in #43 in a follow up.
Comment #50
webchickArgh. I'm still really torn here. I really do not intend to frustrate anyone, but please hear me out:
1) The feature that this is a regression from is actually two parts... the ability to export *and* the ability to import an export. This patch only covers one half. While it seems unfair to force both in the same patch, this feature is relatively useless on its own. It in fact leaves the user at a total dead-end, not at all sure what to do with all of the code they see. I worry they start doing something stupid with it, like over-writing their active config by hand, if we don't give them some guidance.
2) However, providing said guidance is tricky because there is no import, and so in fact the only use for this feature on its own, since we don't support partial config imports, is as a support tool, as Daniel says. However, we generally do not ship core with this kind of tool, leaving that up to the Devel module or similar.
3) However, if we were going to start shipping Drupal with a support tool like this, it makes zero sense to do it just for Views. It should rather be a "Configuration inspector" (or whatever) menu item off the Development menu, and it should work for any config entity.
So I'm honestly not sure what to do here. I can see three possible options:
1) Postpone this patch until a corresponding "import" feature is ready and combine the both of them into one patch.
2) Move this issue to the Devel module.
3) Move it to a centralized location for all config entities.
Not moving down from RTBC just yet, but would love your thoughts.
Comment #51
webchickPatch no longer applies.
Comment #52
mtiftReroll
Comment #54
gddPersonally I would put this into Devel. I agree that without an import the feature doesn't make a ton of sense in core and could end up causing confusion. We aren't going to have a single-file import this release cycle, so that answer isn't really an option.
Comment #55
tim.plunkettDevel would be out backup option. But it might be nice to have the infrastructure in place first...
Here are three patches:
A) is a reroll of #52 after the path/pattern/route_name changes
B) is the code for this patch, but not used by Views (only by the test config entity)
C) is the addition of the interface but no actual changes, since that would be needed for a third-party improvement.
Interdiff is from 52 to A. B and C was just removing more and more code, no interdiff provided.
Comment #56
dawehnerWell, let's be honest providing support for views in core is pretty much hopeless anyway.
Comment #57
tim.plunkettBetter update coming, just posting before I forget.
Comment #59
tim.plunkett#57: config-1898794-57.patch queued for re-testing.
Comment #60
tim.plunkettSee #2099363: Allow single config files to be imported and exported (Resolve regression from Views in Drupal 7) for import. I'll be working on this more too.
Comment #61
tim.plunkettActually, this entire issue was about recreating the UI flow from D7, which is that you export your entity (view) from their own UI (views listing).
The new issue is a single centralized location for everything, and as such, I'm going to mark this a duplicate of the new approach.
It also handles non-config-entity CMI.
Comment #61.0
tim.plunkettUpdated issue summary.