Rails Girls 2015 SoC Banter


(Robin Ward) #74

There’s no good reason why – that pattern emerged well after we started using Ember and never really adopted it. If there’s a good reason to do it, I’d happily accept it :smile:


(Ladydanger) #75

Does anyone else have a black background for discourse meta? Just logged in now and everything’s dark.


(Robin Ward) #76

Yup we’re temporarily dogfooding the dark theme:


(Sarah Ni) #77

I love the dark theme. I think gaming forums would love the mysterious-ness of it all too.


(Sarah Ni) #79

update: figuring out routes so that our ad-plugin/dfp route extension can refresh (direct url)
realizing that we have to create routes that link up with Ember, Rails and Rails Engine? yikes.


(Robin Ward) #80

Let’s add some tests to Purple Tentacle!


(Robin Ward) #81

As discussed on our call last night, I’ve added support for a generic value_list site setting for when you want multiple text boxes but don’t want them to be URLs:

https://github.com/discourse/discourse/commit/b41b176bc6784eee772fed3f413a824b3d58d9cf


(Sarah Ni) #82

Is there a development bug where clicking on the same tab twice causes a cannot read property get of null error?

e.g. clicking Latest twice


(Régis Hanol) #83

Update to the latest version. I already fixed that bug :wink:


(Ladydanger) #84

Hi!

New to qunit testing and had 2 errors if someone has some time to help out.
1st one is unhandled request and 2nd one is element not found. Detailed github issue here: https://github.com/team-melbourne-rgsoc2015/discourse-adplugin/issues/10

Thanks!


(Alan Tan) #85

Hi @ladydanger

Discourse uses the pretender library to mock the server and the error you’re getting is due to pretender not knowing how to handle your route. To see what routes have been declared for testing, have a look at discourse/create-pretender.js.es6 at master · discourse/discourse · GitHub. There is a route declared for plugins but I’ve not used it before so you’ll have to dig a little into that. :smile:

If you’re unsure of what is happening in your acceptance test, I’ve found http://emberjs.com/api/classes/Ember.Test.html#method_pauseTest to be extremely helpful since you can see what is actually happen. :smiley:


(Alan Tan) #86

Ah ok. This is how @eviltrout declared the plugin route.


(Ladydanger) #87

Hey @tgxworld!

First, thank you so much for taking the time to answer :slight_smile: the pretender library makes so much sense.

The question I have with the plugin route is that the purple-tentacle one navigates to plugins/purple-tentacle and it has it’s own route, whereas, our ad plugin doesn’t stipulate a route even though it is a plugin so I’m assuming it’s going to (/admin/plugins/discourse-adplugin).

What our plugin does do is that it uses client.yml to create two categories within site settings, and then uses inputs within site settings to inject inputs made by the user into a script.

en:
  admin_js:
    admin:
      site_settings:
        categories:
          ad_plugin: 'DFP Plugin'
          adsense_plugin: 'Adsense Plugin'

I did visit() the admin/plugin/discourse-adplugin route while playing around with qunit & it looked fine (as in it didn’t return any errors), but I’m confused because our inputs are in the /admin/site_settings/category/:id route and wanted to do fillIn() assertions. What are your thoughts @eviltrout?

Thanks again for looking into it @tgxworld, :ice_cream: really appreciate it! :smiley:

ps. rubybench looks awesome!!!


(Alan Tan) #88

Ah ok. I wasn’t sure about quite a number of things so I decided to dig deeper to understand it.

The reason why visiting /admin/site_settings doesn’t work is because there is nothing handling the following request.

In order to resolve that, we need pretender to be able to handle a call to /admin/site_settings on the “server” by adding a fixture.

Note how the first key in the map declares the URL that is going to be handled and the value is the data that should be return from the server. The fixtures are loaded here.

However…

The regexp we are using right now does not load a fixture that is in a plugin. So in order to make the above work, I actually added the file directly into the fixtures folder in Discourse.


(Ladydanger) #89

this makes it so easy to implement @tgxworld! Can’t wait to try it out today with my pair! Thanks again :+1: :crown:


(Kane York) #90

However, you’re testing the settings page more than your plugin in that case. Because you just plop in the response instead of having it generated from your site_settings.yml.


(Sarah Ni) #91

The officially endorsed Discourse Ad Plugin v0.1 is now available.
Install it now in 3 easy steps.
Includes support features for Google DFP and Adsense. :smile: :sunny:

Coming Soon: Support for Amazon Affiliates.


(Neil Lalonde) #92

Amazing work! :clap: I wrote that other old DFP plugin, and this one is looking like an upgrade.

Something a bit confusing is this setting:

adsense_show_topic_list_top: Disable ad at topic list top location

The setting says “show”, but the functionality is to disable? That checkbox is a bit confusing to me.

I’ll spend more time looking at the code later.


(Ladydanger) #93

Hi Neil,
That reminds me - we forgot to put in the credits for the plugins we referenced! Done now in the Readme! :slight_smile:

With the show to disable, we just negated the logic that so that people could put in the code and the ad would show, and if they wanted to disable it, they could tick the button.

Cheers,
Vi


(Neil Lalonde) #94

I reviewed the code a bit, and it looks good! :+1:

Some issues:

  • I tried it for DFP, but the ads don’t render because the width and height aren’t set when the component is rendered (code here). Looks like you handle this in the adsense and amazon components, so something similar is needed for DFP.

  • Code like this is confusing to read for the point I made earlier:

      if (settings.dfp_topic_list_top_code && !settings.dfp_show_topic_list_top)
    

It actually means “if the code is set and it is showing”, but it looks like “not showing”. Before too many people start using this plugin, that setting name should be changed to “dfp_disable_topic_list_top” or “dfp_hide_topic_list_top”. Using “show” to mean “hide” is confusing. :smile: