search

zinc-collective / convene

An Operating System for the Solidarity Economy
https://convene.zinc.coop
Other
56 stars 19 forks source link

🐞 Clicking "Remove Gizmo 🗑️" button from a `Gizmo`'s edit page does not actually remove the `Gizmo` from the `Section` #1565

Closed zspencer closed 10 months ago

zspencer commented 11 months ago

To Do

Steps to Reproduce:

  1. In a running instance of convene (the base dev instance for now), sign in as "space-member" via regular steps
  2. Go into "System Test"
  3. Click on either edit/settings/configure icon ⚙️
  4. Under "Sections", click on "Listed Room 1"
  5. Under "Gizmos", click the edit icon ⚙️ next to "Markdown Text Block"
  6. Click "Remove Gizmo 🗑️"

Expected Behavior: Return to Edit screen for "Listed Room 1" with "Markdown Text Block" gone (so no Gizmos in this case)

Actual Behavior: Returned to Edit screen for "Listed Room 1" with "Markdown Text Block" still present

Note: Steps 5-6 can be repeated any number of times for a Gizmo with the same results. This also applies to the other Gizmos that have "Remove Gizmo 🗑️" as an option in their configure page: "Livestream" and "Embedded Form"

zspencer commented 11 months ago

@KellyAH or @daltonrpruitt - Are either of you willing to write up some repro steps for this (or a test that isolates it?)

KellyAH commented 11 months ago

I have 5 assigned issues right now. I don't have anymore bandwidth for this one. Can you take it @daltonrpruitt ? Thanks. 🙏 🙇‍♀️

daltonrpruitt commented 11 months ago

Sure thing. Will try do that in a couple hours ish.

daltonrpruitt commented 11 months ago

Steps to reproduce:

  1. In a running instance of convene (the base dev instance for now), sign in as "space-member" via regular steps
  2. Go into "System Test"
  3. Click on either edit/settings/configure icon ⚙️
  4. Under "Sections", click on "Listed Room 1"
  5. Under "Gizmos", click the edit icon ⚙️ next to "Markdown Text Block"
  6. Click "Remove Gizmo 🗑️"

Expected Behavior: Return to Edit screen for "Listed Room 1" with "Markdown Text Block" gone (so no Gizmos in this case) Actual Behavior: Returned to Edit screen for "Listed Room 1" with "Markdown Text Block" still present

Steps 5-6 can be repeated any number of times for a Gizmo with the same results. This also applies to the other Gizmos that have "Remove Gizmo 🗑️" as an option in their configure page: "Livestream" and "Embedded Form"

zspencer commented 11 months ago

These were great! I added some typographical flourishes and put them in the main issue body so it's easier to find without scrolling.

zspencer commented 11 months ago

I'm also going to assign this to myself so I know what to pick up next, but if the spirit moves you and anyone wants to tilt at fixing it; don't let my glowering avatar stop you!

Also, if I get to it before next wednesday, it means Convene will have a system test that @KellyAH can crib off of for https://github.com/zinc-collective/convene/pull/1562!

zspencer commented 10 months ago

@daltonrpruitt - Can you confirm that this is fixed in production when you get a chance and then close this out?

daltonrpruitt commented 10 months ago

I have tried to go through this a few times today, but I keep running into different issues when I try to get to the configure/edit page of my space. The latest one occurs in a fresh instance when I try to go to the configuration/edit page (clicking the gear) for a room I add. The error is shown below. I think it has something to do with some of the changes we were looking at today? I can try to look at it more later. image

Also, my fresh devcontainer instance does not have the two spaces it usually has (System Test and something else). Any insight on this? Is the bin/setup script not doing what I expected it to? Or is something else maybe going on?

KellyAH commented 10 months ago

I have tried to go through this a few times today, but I keep running into different issues when I try to get to the configure/edit page of my space. The latest one occurs in a fresh instance when I try to go to the configuration/edit page (clicking the gear) for a room I add. The error is shown below. I think it has something to do with some of the changes we were looking at today? I can try to look at it more later. image

Also, my fresh devcontainer instance does not have the two spaces it usually has (System Test and something else). Any insight on this? Is the bin/setup script not doing what I expected it to? Or is something else maybe going on?

The error implies there's no space.entrance. 🤔 Maybe no space exists in your dev environment. I think I have to always manually create a space when start from a fresh dev environment.

Lemme delete spaces on my local and see if I can replicate the break.

🤦‍♀️ I also notice I forgot to add space.entrance.furnitures.blank? to the outer if condition when coding https://github.com/zinc-collective/convene/pull/1593

🤔 Maybe I should also add a safe navigational operator too to handle nils. <% if space&.entrance&.furnitures.blank? %>

daltonrpruitt commented 10 months ago

This was after creating a space and trying to go to its edit page. So, there was no entrance, like you said, but the space did exist (I thought, anyways?)

And yeah, that next part makes sense to me, since if you are checking for both whether the entrance exists and if the entrance has furniture, then there is a non-zero chance there is no entrance to fetch furniture for. That operator seems like a good way to avoid more complex/dense branching here. I am not familiar with the syntax, so I'm a little hesitant to say that is definitely set up to do what you want it to, but I'm sure testing or reading the docs more would clear that up.

I will try to look into this more before Wednesday's session. In the meantime, if we want this closed sooner, I will trust anyone saying it's fixed and can gladly close this. (Until I find a way to break it in a different way and have to open another issue or re-open this one for some reason... 😅 🤞 let's hope not)

zspencer commented 10 months ago

Woops! I should have caught the need for a safe navigation operator when reviewing https://github.com/zinc-collective/convene/pull/1593! But yes, tossing a space.entrance&.gizmos.present? should fix this.

zspencer commented 10 months ago

@daltonrpruitt - now that the missing-entrance-causes-boom issue is resolved; if you can retest I would be delighted!

daltonrpruitt commented 10 months ago

It took a few days to get back to this, but I can confirm that the three Gizmos now behave in a way that doesn't allow this problem to occur. Thank you all for the work on this!

zspencer commented 10 months ago

Thank you!!!! Also, don't hesitate to open your own bug reports! If you're not sure which Gizmo or Use Case to connect it to, just comment asking the @zinc-collective/convene-maintainers and we'll take care of it!