ConvDev Index

What does this MR do?

This MR introduces a new page to the admin area that displays statistics how a given GitLab instance compares to other GitLab instances in terms of using the features. See https://gitlab.com/gitlab-org/gitlab-ce/issues/30469 for the complete description and discussion.

Are there points in the code the reviewer needs to double check?

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

• Changelog entry added, if necessary
• [ ] Documentation created/updated will be in a separate MR as per https://gitlab.com/gitlab-org/gitlab-ce/issues/30469#note_30545854
• [ ] API support added
• Tests
  • Added for this feature/bug
  • All builds are passing
• Conform by the merge request performance guides
• Conform by the style guides
• Branch has no merge conflicts with master (if it does - rebase it please)
• Squashed related commits together

What are the relevant issue numbers?

Closes #30469 (closed)

added 312 commits

• d6ba5530...0bce9872 - 311 commits from branch master
• 903bd4f7 - Add model

Compare with previous version

added database label

@yorickpeterse please take a look at the proposed new table. We will add new record to it once a week, because it will only happen after sending usage ping. Hence, the table will be really small. It will be basically append-only and (at least in the current iteration) we will always query for the latest record.

@adamniedzielski Some more context would be appreciated. I have no idea what the purpose is of this table, why the columns have the types they have, etc.

@yorickpeterse the whole context is in the linked issue (https://gitlab.com/gitlab-org/gitlab-ce/issues/30469), but a shorter version for you would be:

1. Look at the mockups
2. Every box contains: leader score, instance score, ratio and has a certain color. version.gitlab.com returns leader score, instance score and level (for color) - we have to store it in the database. We calculate the ratio when rendering the view.
3. Data flow

So to summarise:

1. Only use float if this is a requirement for CE
2. Use an enum (either a Rails enum or a native enum) instead of string values
3. I don't see a benefit for having created_at and updated_at, so unless there's a use-case for these columns I advise removing them.
4. If you want to enforce 1 row per week you'll need some kind of column tracking the year + week, then add a unique index on it. If this doesn't matter then don't bother
5. If possible use a more descriptive table name. I know we're trying to coin a fancy new term here, but I prefer for table names to be more descriptive if possible; though perhaps I might be too pedantic here

@yorickpeterse

3. There's no use case right now, but soon we may start comparing the conversational index across a certain time so I think that leaving these two default fields is fine.

4. It was more for you as an estimate for the size of the table, it's not an official constraint.

changed the description

marked the checklist item add new database table as completed

added 151 commits

• 903bd4f7...837bd6a6 - 150 commits from branch master
• f7362e75 - Add model

Compare with previous version

added 1 commit

• 111c52e5 - Add seed file

Compare with previous version

marked the checklist item add seed file as completed

changed the description

changed the description

marked the checklist item extract a service object from GitlabUsagePingWorker as completed

added 60 commits

• 111c52e5...6cbc69aa - 57 commits from branch master
• 1a682e07 - Add model
• 4cbe4fcd - Add seed file
• 99a345f6 - Extract SubmitUsagePingService from GitlabUsagePingWorker

Compare with previous version

changed the description

marked the checklist item save data from response to the database as completed

@dbalexandre can you do the initial review here? It's only the backend part and I'll still have to integrate it with @psimyn's work in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11440, but I want to get some early feedback.

assigned to @dbalexandre

assigned to @adamniedzielski

@dbalexandre thanks for the code review!

changed title from WIP: ConvDev Index - backend changes to WIP: ConvDev Index

changed the description

marked the checklist item Conform by the merge request performance guides as completed

marked the checklist item Conform by the style guides as completed

added Deliverable ~874211 labels

added 156 commits

• 3d7f23c8...7a509c26 - 154 commits from branch master
• 343f6ad4 - Extract SubmitUsagePingService from GitlabUsagePingWorker
• ffa88b5c - Save data from the response to the database

Compare with previous version

@psimyn can you pick a frontend reviewer? I think that it's nice to start the discussion as early as possible, because it may take a while to go through all the feedback cycles.

@psimyn the Idea to Production steps look like this on my local machine:

I have the impression that they're slightly bigger than in the mockups and not aligned.

changed the description

@annabeldunstone can you please review frontend?

@adamniedzielski I've made changes you mentioned, and updated description with screenshots.

assigned to @annabeldunstone

marked the checklist item Changelog entry added, if necessary as completed

changed the description

resolved all discussions

changed the description

marked the checklist item Added for this feature/bug as completed

@dbalexandre can you please take a look at the backend code again? The new things are:

• Admin::ConversationalDevelopmentIndexMetricsController
• ConversationalDevelopmentIndexMetricsHelper
• ConversationalDevelopmentIndexMetricPresenter

This is looking great!

A few UX things I noticed:

• Each card needs a border-radius (maybe 2px or 3px?)
• the top-border color has some weird angles.
• top border looks a tad too thick

Design	Current

• The steps need a border radius
• They should also have gray SVGs
• They should have box-shadows
• The SVG vertical alignment is slightly off

Design	Current

• There should be more space between the index score percentage and the actual text.
• The text and ? should be gray

Design	Current

@tauriedavis is going to push up some CSS tweaks to get all the values correct. This looks super cool @psimyn!

assigned to @tauriedavis

changed the description

unmarked as a Work In Progress

changed the description

marked the checklist item All builds are passing as completed

changed milestone to %9.3

changed the description

assigned to @annabeldunstone

@annabeldunstone I pushed up my style fixes. Can you take a look again?

@adamniedzielski Thanks I left some notes/questions.

added 410 commits

• 2c0573bd...c72abcef - 409 commits from branch master
• 3d052f05 - Add Conversational Development Index page to admin panel

Compare with previous version

added 1 commit

• 26dde5f5 - Add Conversational Development Index page to admin panel

Compare with previous version

I rebased the branch against the current master. Dividing the changes into multiple meaningful commits was too time consuming (because of later fixups) so I squashed it into the single commit, retaining the authorship as much as possible.

@dbalexandre thanks for the great review and good suggestions! I'm passing to a maintainer for the final review.

@smcgivern can you take a look at this merge request? (but please don't merge before @annabeldunstone checks again the frontend).

added 1 commit

• da1f5255 - Add border radius

Compare with previous version

@psimyn looks awesome! Just a few comments.

assigned to @psimyn

assigned to @smcgivern

@adamniedzielski thanks, this looks really clean! Extra for adding the seeder and factories.

@victorwu have you had a chance to play around with this? You can try different scenarios just by editing the data in your local DB.

assigned to @victorwu

added 2 commits

• ea838141 - Use sum instead of map + inject
• d5df72c1 - Fix copy

Compare with previous version

added 2 commits

• ea838141 - Use sum instead of map + inject
• d5df72c1 - Fix copy

Compare with previous version

@victorwu if you want to test it out locally then executing:

rake db:seed_fu FILTER=conversational_development_index_metrics

will give you a sample record for it.

@psimyn please take a look at the rest of the discussions.

Thanks @adamniedzielski @psimyn : It looks super awesome. The icons are so fun to play with. Buttery smooth.

Everything looks great when I tried it. Tried it without the seed data to see the calculating screen, and tried it with @adamniedzielski's one liner to get the seed data.

@smcgivern : Is there a simple command to get into the Postgres that gdk is running off of?

@tauriedavis : Just noticed that for merge requests, you left out the word created. Is that just to make the words fit on the screen?

assigned to @psimyn

@victorwu gdk psql -d gitlabhq_development should do it!

Yeah, it didn't fit :( Tried to think of alternatives but that was the best I thought of. @victorwu

created per user is another option

Thanks @tauriedavis . Let's leave it. Thanks!

Thanks @smcgivern that worked. I have perfect score now!

added 2 commits

• f922fa52 - fixup some classnames and media queries
• d913ce17 - Merge branch '30469-convdev-index' of gitlab.com:gitlab-org/gitlab-ce into 30469-convdev-index

Compare with previous version

@annabeldunstone thanks for the comments. I've pushed fixes for your comments, let me know if there's any others

Fixes look good to me! Thanks @psimyn

resolved all discussions

Thanks @adamniedzielski @psimyn!

assigned to @smcgivern

approved this merge request

merged

mentioned in commit 3b39cf4e

mentioned in issue #33479 (moved)

mentioned in issue gitlab#10343