[MOD-165][Improvement] Add pending number next to moderation on dashboard #92

laurenbarker · 2017-11-09T20:27:55Z

Purpose

Display pending count next to moderation on dashboard.

Changes

Added request to get the pending number for each provider.
Added skeleton screen for dashboard sidebar.

Before

After

aaxelb

Could cut the new requests in half, if not get rid of them entirely (with a backend change)

aaxelb · 2017-11-09T20:50:59Z

app/components/provider-links/component.js

+    store: service(),
+    theme: service(),
+
+    tagName: 'ul',


These are now a bunch of lists with one item each, which seems weird.

aaxelb · 2017-11-09T21:02:04Z

app/components/provider-links/component.js

+    fetchData: task(function* () {
+        const provider = yield this.get('theme').loadProvider(this.get('provider.id'));
+        const results = yield this.get('store').queryHasMany(provider, 'preprints', {
+            'meta[reviews_state_counts]': true,


The counts are available on the provider, if you use ?related_counts=true.

That only works when fetching one provider at a time, which would be fine, but if we wanted we could make a two-line change to the backend (would have sworn it already worked in the list view, alas) and get all the counts in that one request in load-providers.

Ahhhhh I was looking for that and I never found it...

aaxelb · 2017-11-09T21:09:39Z

app/components/provider-links/component.js

+    },
+
+    fetchData: task(function* () {
+        const provider = yield this.get('theme').loadProvider(this.get('provider.id'));


It feels wrong to use theme on this non-provider-specific view. Could just ask store for it.

Yeah I tried using store first but it didn't work. I'll try again though because it might have been user error.

coveralls · 2017-11-09T21:29:59Z

Coverage decreased (-0.2%) to 18.482% when pulling 01c8724 on laurenbarker:improvement/MOD-165 into 9a54212 on CenterForOpenScience:develop.

laurenbarker · 2017-11-09T21:33:32Z

@aaxelb, I considered changing the backend but we probably won't be able to get that released for a long time. What about adding a ticket for the backend changes in the next sprint as an improvement to this?

aaxelb · 2017-11-09T21:35:08Z

@laurenbarker Sounds good to me. Depending how fast it loads with one request per provider, it might not be important to optimize.

laurenbarker · 2017-11-13T21:09:27Z

I'm not sure how to write tests for components that use ember-concurrency... I think it would involve refactoring the component so that the task is passed in. That way the task can be overwritten in tests.

coveralls · 2017-11-13T22:15:16Z

Coverage decreased (-0.1%) to 18.519% when pulling 8a114eb on laurenbarker:improvement/MOD-165 into 9a54212 on CenterForOpenScience:develop.

aaxelb

Tiny nitpicks, otherwise 🇫🇯

aaxelb · 2017-11-14T18:47:19Z

app/styles/_util.scss

@@ -2,3 +2,11 @@
  pointer-events: none;
  opacity: 0.4;
 }
+
+.unread-label {
+    background-color: $color-bg-gray-light;


Are we back to 4-space indents for CSS?

Oops... We should add a linter for that.

aaxelb · 2017-11-14T18:48:10Z

app/components/provider-links/style.scss

@@ -0,0 +1,42 @@
+& {


Doesn't ember-component-css take care of making sure these styles only apply to this component? Do we need to wrap everything in & {}?

It's not for scoping; it allows you to set css on component. So really the only thing that needs to go in & {} is the styling for the actual component. Should I pull everything else out?

Ah, right, I skimmed right over the styles and just saw the classes. I guess it seems cleaner to use & only for the styles on the component element itself, and pull everything else out, but that's not a strong preference.

@aaxelb, they have been un-nested.

coveralls · 2017-11-14T19:34:33Z

Coverage decreased (-0.1%) to 18.519% when pulling aa75d36 on laurenbarker:improvement/MOD-165 into 9a54212 on CenterForOpenScience:develop.

coveralls · 2017-11-14T20:01:47Z

Coverage decreased (-0.1%) to 18.519% when pulling dcf90cb on laurenbarker:improvement/MOD-165 into 9a54212 on CenterForOpenScience:develop.

aaxelb

I lied, I found a couple more nits to pick. Otherwise, 🇻🇺

aaxelb · 2017-11-14T20:20:29Z

app/components/provider-links/template.hbs

+            <li>
+                {{#link-to 'preprints.provider.moderation' provider.id}}
+                    {{t 'global.moderation'}}
+                {{/link-to}}


FYI, because you took the time to remove the underline on the moderation count, you could get rid of this extended underline:

by stripping the whitespace inside the link-to with strategic use of tildes

aaxelb · 2017-11-14T20:24:04Z

app/components/provider-links/template.hbs

+    {{#content-placeholders as |placeholder|}}
+        <span>
+            <i class="fa-li icon--skeleton">{{placeholder.icon}}</i>
+            {{placeholder.heading subtitle=false}}


It looks like it's still adding a subtitle, and it looks weird:

Did you run yarn install? I made a change to ember-content-placeholders.

Oh whoops, I did not. All good 👍

aaxelb · 2017-11-14T20:28:04Z

Oh, and add this to the changelog, please!

Update changelog

coveralls · 2017-11-14T20:48:22Z

Coverage decreased (-0.1%) to 18.519% when pulling a740cb5 on laurenbarker:improvement/MOD-165 into 9a54212 on CenterForOpenScience:develop.

aaxelb

🇹🇻

laurenbarker force-pushed the improvement/MOD-165 branch from c6c6e0b to 48d0436 Compare November 9, 2017 20:31

Add pending number next to moderation on dashboard

01c8724

laurenbarker force-pushed the improvement/MOD-165 branch from 48d0436 to 01c8724 Compare November 9, 2017 20:38

aaxelb requested changes Nov 9, 2017

View reviewed changes

laurenbarker added 2 commits November 13, 2017 15:07

Use related_counts instead of queryHasMany

7a28d0e

Refactor list

8a114eb

laurenbarker force-pushed the improvement/MOD-165 branch from b6540f3 to 8a114eb Compare November 13, 2017 20:57

aaxelb requested changes Nov 14, 2017

View reviewed changes

Fix css indentation

aa75d36

Un-nest css

dcf90cb

aaxelb requested changes Nov 14, 2017

View reviewed changes

Remove excess whitespace

a740cb5

Update changelog

aaxelb approved these changes Nov 14, 2017

View reviewed changes

aaxelb merged commit 7a8b800 into CenterForOpenScience:develop Nov 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MOD-165][Improvement] Add pending number next to moderation on dashboard #92

[MOD-165][Improvement] Add pending number next to moderation on dashboard #92

laurenbarker commented Nov 9, 2017

aaxelb left a comment

aaxelb Nov 9, 2017

aaxelb Nov 9, 2017

laurenbarker Nov 9, 2017

aaxelb Nov 9, 2017

laurenbarker Nov 9, 2017

coveralls commented Nov 9, 2017

laurenbarker commented Nov 9, 2017

aaxelb commented Nov 9, 2017

laurenbarker commented Nov 13, 2017

coveralls commented Nov 13, 2017

aaxelb left a comment

aaxelb Nov 14, 2017

laurenbarker Nov 14, 2017

aaxelb Nov 14, 2017

laurenbarker Nov 14, 2017

aaxelb Nov 14, 2017

laurenbarker Nov 14, 2017

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

aaxelb left a comment

aaxelb Nov 14, 2017

aaxelb Nov 14, 2017

laurenbarker Nov 14, 2017

aaxelb Nov 14, 2017

aaxelb commented Nov 14, 2017

coveralls commented Nov 14, 2017

aaxelb left a comment

[MOD-165][Improvement] Add pending number next to moderation on dashboard #92

[MOD-165][Improvement] Add pending number next to moderation on dashboard #92

Conversation

laurenbarker commented Nov 9, 2017

Purpose

Changes

Before

After

aaxelb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 9, 2017

laurenbarker commented Nov 9, 2017

aaxelb commented Nov 9, 2017

laurenbarker commented Nov 13, 2017

coveralls commented Nov 13, 2017

aaxelb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 14, 2017

coveralls commented Nov 14, 2017

aaxelb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaxelb commented Nov 14, 2017

coveralls commented Nov 14, 2017

aaxelb left a comment

Choose a reason for hiding this comment