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

Adds a new admin setting for experience levels. #10870

Merged

Conversation

AfroDevGirl
Copy link
Contributor

@AfroDevGirl AfroDevGirl commented Oct 15, 2020

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

These admin settings are used in the post management UI to determine the range of experience. These values fallback to the ones we currently see in DEV, "Total Newbies" and "Senior Devs". There is also a removal of the word coding when talking about experience level, since the context for each forem instance should be implied when referencing experience.

Related Tickets & Documents

Closes #10845

QA Instructions, Screenshots, Recordings

  • Verify that the fallback values of 'Total Newbies' and 'Senior Devs' are present before testing the new functionality
  • As an admin with a config update role navigate to /admin/config
  • Toggle the Community Content section and observe there are two new fields at the bottom, Experience Low/High
  • Update those values with any text and save the config
  • Then, create a post and publish it
  • In your personal dashboard, click manage to view a post's experience level and observe that the saved config values are there. There should be no changes in setting the experience level of a post.
  • Navigate to /settings/ux and observe that under Content Customization the text now reads What is your approximate experience level (1-5)?

UX Settings:

Screen Shot 2020-10-15 at 3 26 01 PM

Admin Config:

Screen Shot 2020-10-15 at 3 26 32 PM

Post Management:

Screen Shot 2020-10-15 at 3 27 00 PM

Added tests?

  • yes
  • no, because they aren't needed
  • no, because I need help

Added to documentation?

  • docs.forem.com
  • readme
  • no documentation needed

[optional] Are there any post deployment tasks we need to perform?

Edit: We need to add the values we want to display in DEV's admin dashboard to keep "Senior Devs" instead of the newly included defaults.

[optional] What gif best describes this PR or how it makes you feel?

kim kardashian west saying "I wish everyone could experience this"

This is used in the post management UI to determine the range of experience. These values fallback to the ones we currently see in DEV, Total Newbies and Senior Devs. There is also a removal of the word coding when talking about experience level, since the context for each forem instance should be implied when referencing experience.
@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 15, 2020
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2020

CLA assistant check
All committers have signed the CLA.

<button value="1" name="rating_vote[rating]" class="level-rating-button <%= "selected" if @rating_vote&.rating == 1.0 %>">1</button>
<button value="2" name="rating_vote[rating]" class="level-rating-button <%= "selected" if @rating_vote&.rating == 2.0 %>">2</button>
<button value="3" name="rating_vote[rating]" class="level-rating-button <%= "selected" if @rating_vote&.rating == 3.0 %>">3</button>
<button value="1" name="rating_vote[rating]" class="level-rating-button <%= "selected" if @rating_vote&.rating.to_d == 1.0.to_d %>">1</button>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was getting a rubocop issue on these. I'm happy to revert this particular change if y'all would prefer that.

Copy link
Contributor

@fdocr fdocr left a comment

Choose a reason for hiding this comment

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

Thanks for taking a stab at this @AfroDevGirl!

I'm leaving a small implementation comment inline, which in general looks great to me. From the product context though @benhalpern may have some context or any other notes (if any)

app/models/site_config.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@benhalpern benhalpern left a comment

Choose a reason for hiding this comment

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

This looks good to me. Definitely helps this feature immediately become more relevant for different Forems.

I have a thought about generalizing this in the future moreso but I think this is a good stepping stone.

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 16, 2020
Copy link
Contributor

@fdocr fdocr left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for including the requested changes @AfroDevGirl.

The PR ran into some conflicts probably due to some recent edits to that file by other commits. Would you mind fixing those in order to merge this? Thanks!

@pr-triage pr-triage bot added PR: reviewed-changes-requested bot applied label for PR's where reviewer requests changes and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 16, 2020
@AfroDevGirl AfroDevGirl requested a review from a team October 16, 2020 16:43
@pr-triage pr-triage bot added PR: unreviewed bot applied label for PR's with no review and removed PR: reviewed-changes-requested bot applied label for PR's where reviewer requests changes labels Oct 16, 2020
Copy link
Contributor

@fdocr fdocr left a comment

Choose a reason for hiding this comment

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

👍🏼

@pr-triage pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 16, 2020
@fdocr fdocr merged commit f4a10e4 into forem:master Oct 16, 2020
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 16, 2020
boardfish pushed a commit to boardfish/forem that referenced this pull request Oct 17, 2020
* Adds a new admin setting for experience levels.

This is used in the post management UI to determine the range of experience. These values fallback to the ones we currently see in DEV, Total Newbies and Senior Devs. There is also a removal of the word coding when talking about experience level, since the context for each forem instance should be implied when referencing experience.

* Cleans up experience defaults

* Fixes instance_of usage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: merged bot applied label for PR's that are merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experience Level of Post range should be configurable
4 participants