zinc-collective / convene

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

Refactor Jitsi videobridge as Furniture #191

Closed zspencer closed 3 years ago

zspencer commented 3 years ago

See https://github.com/zinc-collective/convene/issues/186

In order to support more diverse use cases, I'm trying to move the Videobridge into a piece of Furniture that can be placed in a Room. The intent here is to allow us to have Rooms that don't use a VideoBridge, such as for the Fundraising ACH Tool

Can Haz Review on:

  1. [x] Running bin/setup completes successfully?
  2. [x] Running convene-web/bin rake release:after_build makes sure all the rooms on your machine have a videobridge?
  3. [x] Sanity check?
anaulin commented 3 years ago

To clarify, by "ComLink" you mean the video call feature that is right now the main thing that Convene offers, right?

If so, I really like the idea of moving it into a piece of furniture, instead of having it be a "special citizen". It really helps me understand that Convene is a modular space to get together with others, and that I can choose what to put in that space -- maybe it is video chat, maybe it is a "check this bank account" thing, maybe it is text chat, etc.

anaulin commented 3 years ago

I also gave the code a read, and so far it makes sense to me. The new Blueprint model is clear as the entrypoint through which a space and its furniture get assembled and configured as per clients' specifications. As a former coworker used to say: I dig.

anaulin commented 3 years ago

Marinating on this a bit more, I'm wondering if "comlink" is a confusing name for video call, especially if we might end up having a Jitsi version and BBB version (at least for a little while). Would "video chat" as a name be overly specific? (e.g. BBB also has integrated text chat and whatnot).

zspencer commented 3 years ago

How about comlink-jitsi orvideobridge-bbb? That way it starts with the value prop (comlink/videobridge) and the highly-pertinent-but-not-quite-the-point partnership details are still enshrined as a kind of "natural primary key?"

anaulin commented 3 years ago

I like that, I think either option would work. Not sure how self-explanatory is "comlink", but it is shorter than "videobridge". Curious if @user512 and/or @roseaboveit have a take on the naming dilemma.

zspencer commented 3 years ago

I'm also cool with sticking with videobridge :).

anaulin commented 3 years ago

[ETA: This might have been due to some subtle difference in space configurations, because after resetting the DB once, all subsequent runs of bin/setup worked fine.]

Something in the idempotence of bin/setup seems to have broken (maybe because the find_or_create! in Blueprint isn't quite idempotent?)

I got the error:

[...]
== Seeding development database according to feature settings in .env ==
Running via Spring preloader in process 13960
rake aborted!
ActiveRecord::RecordInvalid: Validation failed: Branded domain has already been taken, Name has already been taken
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/validations.rb:80:in `raise_validation_error'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/validations.rb:53:in `save!'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:318:in `block in save!'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:375:in `block in with_transaction_returning_status'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:278:in `transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:212:in `transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:366:in `with_transaction_returning_status'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:318:in `save!'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/suppressor.rb:48:in `save!'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/persistence.rb:635:in `block in update!'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:375:in `block in with_transaction_returning_status'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `block in transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/transaction.rb:280:in `block in within_new_transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
/Users/anaulin/.gem/ruby/2.7.1/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
/Users/anaulin/.gem/ruby/2.7.1/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
/Users/anaulin/.gem/ruby/2.7.1/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
/Users/anaulin/.gem/ruby/2.7.1/gems/activesupport-6.0.3.4/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/transaction.rb:278:in `within_new_transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/connection_adapters/abstract/database_statements.rb:280:in `transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:212:in `transaction'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/transactions.rb:366:in `with_transaction_returning_status'
/Users/anaulin/.gem/ruby/2.7.1/gems/activerecord-6.0.3.4/lib/active_record/persistence.rb:633:in `update!'
/Users/anaulin/src/github.com/zinc-collective/convene/convene-web/app/models/blueprint.rb:11:in `find_or_create!'
/Users/anaulin/src/github.com/zinc-collective/convene/convene-web/app/models/system_test_space.rb:11:in `prepare'
anaulin commented 3 years ago

✅ Successfully ran bin/setup after a bin/rails db:reset. After this step, all the rooms in the system-test space had a working videobridge. ✅ Successfully ran bin/rake release:after_build. Rooms have a working videobridge after this, but they already had it before, so not sure if it did what it was supposed to do.

anaulin commented 3 years ago

It seems like the error I got in my first run of bin/setup might have been due to some slightly-different configurations of the space? After running db:reset once, several runs of bin/setup succeeded.