Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frontend changes for showing tabbed course overview stats #5064

Merged
merged 23 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
02da8da
Add namespace-id mapping and make required modifications.
vaidehi44 Jul 21, 2022
8c237a3
Add components to display namespace overview stats in tabs.
vaidehi44 Jul 21, 2022
a0bb519
Modify WikidataOverviewStats container class to adjust css.
vaidehi44 Jul 21, 2022
0c81559
Merge remote-tracking branch 'upstream/master' into namespace-stats-ui
vaidehi44 Jul 21, 2022
23821ba
Some eslint fixes.
vaidehi44 Jul 21, 2022
918faf0
Move stats title function to article utils.
vaidehi44 Jul 23, 2022
8f2d6ca
Refined some code.
vaidehi44 Jul 23, 2022
9cc4b44
Test for overview stats title util function.
vaidehi44 Jul 23, 2022
5670769
Minor changes
vaidehi44 Jul 24, 2022
7dd7f26
Lint fixes
vaidehi44 Jul 24, 2022
f0e4e0b
Fix ruby spec test.
vaidehi44 Jul 24, 2022
c0b47bc
New stats content component.
vaidehi44 Jul 26, 2022
ae7d15d
New UI for tabs.
vaidehi44 Jul 26, 2022
f019e01
Merge branch 'master' of https://github.com/WikiEducationFoundation/W…
vaidehi44 Jul 26, 2022
d8d7bf0
Add method to get namespace id from title.
vaidehi44 Jul 27, 2022
49b696b
Write ruby test for tabbed course stats
vaidehi44 Jul 28, 2022
f7c9833
Change tab id
vaidehi44 Jul 28, 2022
c170b1c
Update format method for all course stats
vaidehi44 Jul 28, 2022
82d042f
Merge remote-tracking branch 'upstream/master' into namespace-stats-ui
vaidehi44 Jul 28, 2022
8aea637
Update renderZero prop for all namespace attributes
vaidehi44 Jul 28, 2022
1639e9f
Update feature test
vaidehi44 Jul 29, 2022
34b8c98
Handle rendering of tabs if there's only one stats object
vaidehi44 Jul 30, 2022
6f58d37
Fix some Lint errors
vaidehi44 Jul 30, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,28 @@ const NamespaceOverviewStats = ({ statistics }) => {
className="stat-display__value"
stat = {statistics.edited_count}
statMsg={I18n.t('metrics.articles_edited')}
renderZero={true}
renderZero={false}
/>
<OverviewStat
id="total-edits"
className="stat-display__value"
stat = {statistics.revision_count}
statMsg={I18n.t('metrics.edit_count_description')}
renderZero={true}
renderZero={false}
/>
<OverviewStat
id="student-editors"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the user count needs to be in any namespace-specific stats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't it give insight of how many students worked on particular tracked namespace?

className="stat-display__value"
stat={statistics.user_count}
statMsg={I18n.t('courses.student_editors')}
renderZero={true}
renderZero={false}
/>
<OverviewStat
id="word-count"
className="stat-display__value"
stat={statistics.word_count}
statMsg={I18n.t('metrics.word_count')}
renderZero={true}
renderZero={false}
/>
<OverviewStat
id="references-count"
Expand All @@ -52,7 +52,7 @@ const NamespaceOverviewStats = ({ statistics }) => {
className="stat-display__value"
stat={statistics.views_count}
statMsg={I18n.t('metrics.view_count_description')}
renderZero={true}
renderZero={false}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import PropTypes from 'prop-types';
const OverviewStatsTab = ({ id, title, active, onClick }) => {
const isActive = (active) ? ' active' : '';
const tabClass = `tab${isActive}`;
const tabId = `tab-${id}`;
return (
<div className={tabClass} onClick={onClick} id={id}>
<div className={tabClass} onClick={onClick} id={tabId}>
<p>{title}</p>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ const OverviewStatsTabs = ({ statistics }) => {
const [currentTabId, setCurrentTabId] = useState(0);

const onTabChange = (e) => {
return setCurrentTabId(Number(e.currentTarget.id));
const tabId = e.currentTarget.id;
const tabIdNumber = Number(tabId.split('-')[1]);
return setCurrentTabId(tabIdNumber);
};

const statsList = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import StudentRevisionRow from './StudentRevisionRow';

// Libraries
import CourseUtils from '~/app/assets/javascripts/utils/course_utils.js';
import ArticleUtils from '~/app/assets/javascripts/utils/article_utils.js';
import studentListKeys from '@components/students/shared/StudentList/student_list_keys.js';

export class StudentRevisionsList extends React.Component {
constructor(props) {
super(props);
this.state = {
isOpen: false,
namespace: -1 // show all namespace revisions by default
namespace: 'all' // show all namespace revisions by default
};

this.toggleDrawer = this.toggleDrawer.bind(this);
Expand All @@ -24,7 +25,7 @@ export class StudentRevisionsList extends React.Component {
onNamespaceChange = (e) => {
// Open the drawer when filter is used
if (!this.state.isOpen) this.setState({ isOpen: true });
return this.setState({ namespace: Number(e.target.value) });
return this.setState({ namespace: e.target.value });
};

// filter the revisions according to namespace
Expand All @@ -33,10 +34,11 @@ export class StudentRevisionsList extends React.Component {

let revisions = [];
if (userRevisions[student.id] !== undefined && userRevisions[student.id] !== null) {
revisions = (this.state.namespace === -1)
revisions = (this.state.namespace === 'all')
? userRevisions[student.id]
: userRevisions[student.id].filter((rev) => {
return rev.article.namespace === this.state.namespace;
const current_ns_id = ArticleUtils.getNamespaceId(this.state.namespace);
return rev.article.namespace === current_ns_id;
});
}
return revisions;
Expand Down Expand Up @@ -97,10 +99,10 @@ export class StudentRevisionsList extends React.Component {
value={this.state.namespace}
onChange={this.onNamespaceChange}
>
<option value={-1}>{I18n.t('namespace.all')}</option>
<option value={0}>{I18n.t('namespace.article')}</option>
<option value={2}>{I18n.t('namespace.user')}</option>
<option value={1}>{I18n.t('namespace.talk')}</option>
<option value={'all'}>{I18n.t('namespace.all')}</option>
<option value={'main'}>{I18n.t('namespace.main')}</option>
<option value={'user'}>{I18n.t('namespace.user')}</option>
<option value={'talk'}>{I18n.t('namespace.talk')}</option>
</select>
);
return (
Expand Down
7 changes: 7 additions & 0 deletions app/assets/javascripts/utils/article_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,11 @@ export default class ArticleUtils {
wikiversity: 'collection'
}
};

// Get namespace id from its title
static getNamespaceId(namespace) {
const ns_id_mapping = this.NamespaceIdMapping;
const ns_id = Object.keys(ns_id_mapping).find(ns => ns_id_mapping[ns] === namespace);
return Number(ns_id);
}
}
15 changes: 9 additions & 6 deletions app/helpers/course_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,15 @@ def course_i18n(message_key, course = nil)
I18n.t("#{string_prefix}.#{message_key}")
end

def format_wikidata_stats(course_stats)
course_stats['www.wikidata.org']['other updates'] += course_stats['www.wikidata.org']['unknown']
course_stats['www.wikidata.org'].reject! { |k, _v| k == 'unknown' }
course_stats.each_value do |stat|
stat.each do |key, value|
stat[key] = number_to_human(value)
def format_course_stats(course_stats)
course_stats.each do |wiki_ns_key, wiki_ns_stats|
if wiki_ns_key == 'www.wikidata.org'
course_stats[wiki_ns_key]['other updates'] += course_stats[wiki_ns_key]['unknown']
course_stats[wiki_ns_key].reject! { |k, _v| k == 'unknown' }
end
# convert stats to human readable values
course_stats[wiki_ns_key].each do |stat_key, stat|
course_stats[wiki_ns_key][stat_key] = number_to_human(stat)
end
end
course_stats
Expand Down
4 changes: 2 additions & 2 deletions app/presenters/courses_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ def trained_percent
end

def wikidata_stats
stats ||= courses.joins(:course_stat).where.not(course_stats: nil).map do |course|
stats ||= courses.joins(:course_stat).where.not(course_stats: nil).map { |course|
course.course_stat.stats_hash['www.wikidata.org']
end
}.compact
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think rubocop will complain about using braces for a multi-line block instead of do/end. You can still chain the do/end syntax with end.compact.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohkk, thanks.

return { 'www.wikidata.org' => stats.inject { |a, b| a.merge(b) { |_, x, y| x + y } } }
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/campaigns/show.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if @campaign
json.created_at @campaign.created_at

if @presenter.wikidata_stats['www.wikidata.org']
json.course_stats format_wikidata_stats(@presenter.wikidata_stats)
json.course_stats format_course_stats(@presenter.wikidata_stats)
end
end
end
2 changes: 1 addition & 1 deletion app/views/courses/course.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ json.course do
json.wiki_string_prefix @course.home_wiki.string_prefix

if @course.course_stat
@course.course_stat.stats_hash = format_wikidata_stats(@course.course_stat.stats_hash)
@course.course_stat.stats_hash = format_course_stats(@course.course_stat.stats_hash)
json.course_stats @course.course_stat, :id, :stats_hash
end

Expand Down
45 changes: 45 additions & 0 deletions spec/features/detailed_course_stats_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

require 'rails_helper'

describe 'Detailed course overview stats', type: :feature, js: true do
let(:course) { create(:course, start: '2022-01-01', end: '2023-01-01') }
before do
create(:course_stats, course: course,
stats_hash: {
'www.wikidata.org': {
'claims created': 35,
'other updates': 2,
'unknown': 6
},
'en.wikibooks.org-namespace-102': {
'new_count': 5,
'edited_count': 72
},
'en.wikipedia.org-namespace-0': {
'new_count': 16,
'edited_count': 103
}
}
)
visit "/courses/#{course.slug}"
end

it 'shows tabs for all the course stats objects' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be a little better to condense this into one test example, because only loading the page once will be a little faster, and all these tasks together represent a typical user flow: first load the page and see the expected tabs, then look at the default tab's content and confirm that at least one of the expected stats is being displayed (eg, 35 claimed created), then click to the next tab (ideally identifying it by the rendered label rather than the tab-1 id) and verify that the wikidata stats are gone and the new stats are there, then clicking the third tab and doing the same thing.

expect(page.find('#tab-0')).to have_content('www.wikidata.org')
expect(page.find('#tab-1')).to have_content('en.wikibooks.org - Cookbook')
expect(page.find('#tab-2')).to have_content('en.wikipedia.org - Mainspace')
end

it 'shows cookbook stats on clicking the \'en.wikibooks.org - Cookbook\' tab' do
page.find('#tab-1').click
expect(page).to have_content('en.wikibooks.org - Cookbook')
expect(page).to have_content('Articles Edited')
end

it 'shows wikidata overview stats on clicking the \'www.wikidata.org\' tab' do
page.find('#tab-0').click
expect(page).to have_content('www.wikidata.org')
expect(page).to have_content('Claims')
end
end