DEV: Plugin API to add directory columns (take 2) (PR #13440)

GitHub

The previous PR I did for this, used the preload store. This is now an ajax request

No preload store anymore. Now we have this like I mentioned above

Kinda weird method name here… but we have 2 indexes. One is public index and one is edit index so I went with it.

        expect(DirectoryColumn.find_by(name: column_name, icon: 'recycle', enabled: false)).to be_present

Very minor change but I feel like it reads better.

This is a sign that you should create a new controller. It maps to your URLs below too. You could have DirectoryColumnsController and EditDirectoryColumnsController.

This feels a bit messy to me. Can you explain more about what you mean by “ensures that no test will run and change the total number of registrations”. I would really prefer if we didn’t have to delete stuff for tests to pass.

This is the error with the line removed

1) Plugin::Instance#add_directory_column with valid column name creates a directory column record when directory items are refreshed
     Failure/Error: expect(before_event_count).to eq(after_event_count), "DiscourseEvent registrations were not cleaned up"
       DiscourseEvent registrations were not cleaned up
     # ./spec/rails_helper.rb:281:in `block (2 levels) in <top (required)>'
  

Caused by discourse/rails_helper.rb at 83a6ad32ffe75ae222028feddeca169fc5be54ac · discourse/discourse · GitHub

I see, thanks. I would prefer if you weren’t reaching into events to delete manually and we had a method for this.

Normally a test calls .off to remove an event it wires up. Perhaps you could add .all_off and call that?

Yeah, that’s a really good solution. Pushed a commit that adds DiscourseEvent#all_off

thanks, looks good now.

This pull request has been mentioned on Discourse Meta. There might be relevant details there: