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

Adding pre-commit hook for linting #453

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 19 additions & 10 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,27 @@
# $ git commit --no-verify
# $ git commit -n

# Check if there are any staged changes
if git diff --cached --quiet; then
echo "No changes to commit"
exit 0
fi

echo "Running linter..."
bundle exec rake standard
(cd app && bundle exec rubocop --autocorrect-all)
linter_status=$?

if [ $linter_status -ne 0 ]; then
echo "Fix above before committing. Run 'git commit -n' to bypass linter."
echo "Fix remaining linting issues before committing. Run 'git commit -n' to bypass linter."
exit 1
fi

# Stage the files that were auto-corrected by RuboCop
git add app/**/*.rb
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unexpected to me, and I'd guess will cause people to inadvertently commit files. Also, this script should really make sure it's running in the right directory, ideally by doing something like:

cd "$(git rev-parse --show-toplevel)"

at the top of the script. (This will cd to the top-level of the git repo.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch


echo "Running JavaScript/TypeScript formatter..."
(cd app && npm run format)

echo "Running Terraform formatter"
files=$(git diff --cached --name-only terraform)
for f in $files
Expand All @@ -37,18 +49,15 @@ COMMIT_MSG_FILE=$1
# Get the current branch name
BRANCH_NAME=$(git symbolic-ref --short HEAD)

# Extract the ticket number from the branch name
# This sed command looks for 'ffs' followed by an optional single character delimiter,
# then captures all following digits
TICKET=$(echo $BRANCH_NAME | sed -E 's/.*ffs[-_]?([0-9]+).*/\1/' | tr '[:lower:]' '[:upper:]')

if [ -n "$TICKET" ]; then
# Only proceed if branch starts with ffs- or FFS- followed by numbers
if [[ $BRANCH_NAME =~ ^[Ff][Ff][Ss]-([0-9]+) ]]; then
TICKET="${BASH_REMATCH[1]}"

# Read the current commit message
COMMIT_MSG=$(cat $COMMIT_MSG_FILE)

# Check if the commit message already starts with the ticket number
if [[ $COMMIT_MSG != FFS-$TICKET:* ]]; then
# Prepend the ticket number to the commit message
sed -i.bak "1s/^/FFS-$TICKET: /" $COMMIT_MSG_FILE
sed -i.bak '1s|^|FFS-'"$TICKET"': |' "$COMMIT_MSG_FILE"
fi
fi
7 changes: 7 additions & 0 deletions app/.prettierrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"semi": false,
"singleQuote": true,
"trailingComma": "es5",
"printWidth": 100,
"tabWidth": 2
}
20 changes: 10 additions & 10 deletions app/app/javascript/application.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
// Entry point for the build script in your package.json
import "@hotwired/turbo-rails"
import "./controllers"
import '@hotwired/turbo-rails'
import './controllers'

import "@uswds/uswds"
import '@uswds/uswds'

// make sure USWDS components are wired to their behavior after a Turbo navigation
import components from "@uswds/uswds/src/js/components"
let initialLoad = true;
import components from '@uswds/uswds/src/js/components'
let initialLoad = true

document.addEventListener("turbo:load", () => {
document.addEventListener('turbo:load', () => {
if (initialLoad) {
// initial domready is handled by `import "uswds"` code
initialLoad = false
Expand All @@ -20,8 +20,8 @@ document.addEventListener("turbo:load", () => {
const behavior = components[key]
behavior.on(target)
})
});
})

document.addEventListener("turbo:frame-render", () => {
initialLoad = true;
});
document.addEventListener('turbo:frame-render', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, can we just standardize on double-quotes in JS for consistency with Ruby? Is there a technical reason to use single quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No technical reason. By convention I'm used to single quotes and only using double quotes for strings that require interpolation. It's simply a practice that I've used for years now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think string interpolation in JS uses tildes, right?

Copy link
Contributor Author

@GeorgeCodes19 GeorgeCodes19 Feb 18, 2025

Choose a reason for hiding this comment

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

JS uses backticks. In other languages that differs (PHP for example uses double quotes)- it's just convention that I'm used to, but not married to. I don't mind setting "singleQuote": false. Double quotes do allow us to avoid escaping apostrophes though.

initialLoad = true
})
4 changes: 2 additions & 2 deletions app/app/javascript/controllers/application.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Application } from "@hotwired/stimulus"
import { Application } from '@hotwired/stimulus'

const application = Application.start()

// Configure Stimulus development experience
application.debug = false
window.Stimulus = application
window.Stimulus = application

export { application }
96 changes: 46 additions & 50 deletions app/app/javascript/controllers/cbv/employer_search.js
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
import { Controller } from "@hotwired/stimulus"
import { loadPinwheel, initializePinwheel } from "../../utilities/pinwheel"
import { fetchToken } from '../../utilities/api';
import { trackUserAction } from '../../utilities/api';
import { Controller } from '@hotwired/stimulus'
import { loadPinwheel, initializePinwheel } from '../../utilities/pinwheel'
import { fetchToken } from '../../utilities/api'
import { trackUserAction } from '../../utilities/api'

export default class extends Controller {
static targets = [
"form",
"userAccountId",
"employerButton"
];
static targets = ['form', 'userAccountId', 'employerButton']

static values = {
cbvFlowId: Number
cbvFlowId: Number,
}

pinwheel = loadPinwheel();
pinwheel = loadPinwheel()

connect() {
this.errorHandler = document.addEventListener("turbo:frame-missing", this.onTurboError)
this.errorHandler = document.addEventListener('turbo:frame-missing', this.onTurboError)
}

disconnect() {
document.removeEventListener("turbo:frame-missing", this.errorHandler)
document.removeEventListener('turbo:frame-missing', this.errorHandler)
}

onTurboError(event) {
console.warn("Got turbo error, redirecting:", event)
console.warn('Got turbo error, redirecting:', event)

const location = event.detail.response.url
event.detail.visit(location)
Expand All @@ -36,87 +32,87 @@ export default class extends Controller {
if (eventName === 'success') {
const { accountId } = eventPayload
this.userAccountIdTarget.value = accountId
trackUserAction("PinwheelSuccess", {
trackUserAction('PinwheelSuccess', {
account_id: eventPayload.accountId,
platform_id: eventPayload.platformId
platform_id: eventPayload.platformId,
})
this.formTarget.submit();
this.formTarget.submit()
} else if (eventName === 'screen_transition') {
const { screenName } = eventPayload

switch (screenName) {
case "LOGIN":
trackUserAction("PinwheelShowLoginPage", {
case 'LOGIN':
trackUserAction('PinwheelShowLoginPage', {
screen_name: screenName,
employer_name: eventPayload.selectedEmployerName,
platform_name: eventPayload.selectedPlatformName
platform_name: eventPayload.selectedPlatformName,
})
break
case "PROVIDER_CONFIRMATION":
trackUserAction("PinwheelShowProviderConfirmationPage", {})
case 'PROVIDER_CONFIRMATION':
trackUserAction('PinwheelShowProviderConfirmationPage', {})
break
case "SEARCH_DEFAULT":
trackUserAction("PinwheelShowDefaultProviderSearch", {})
case 'SEARCH_DEFAULT':
trackUserAction('PinwheelShowDefaultProviderSearch', {})
break
case "EXIT_CONFIRMATION":
trackUserAction("PinwheelAttemptClose", {})
case 'EXIT_CONFIRMATION':
trackUserAction('PinwheelAttemptClose', {})
break
}
} else if (eventName === 'login_attempt') {
trackUserAction("PinwheelAttemptLogin", {})
trackUserAction('PinwheelAttemptLogin', {})
} else if (eventName === 'error') {
const { type, code, message } = eventPayload
trackUserAction("PinwheelError", { type, code, message })
trackUserAction('PinwheelError', { type, code, message })
} else if (eventName === 'exit') {
trackUserAction("PinwheelCloseModal", {})
trackUserAction('PinwheelCloseModal', {})
this.showHelpBanner()
}
}

getDocumentLocale() {
const docLocale = document.documentElement.lang;
if (docLocale) return docLocale;
const docLocale = document.documentElement.lang
if (docLocale) return docLocale
// Extract locale from URL path (e.g., /en/cbv/employer_search)
const pathMatch = window.location.pathname.match(/^\/([a-z]{2})\//i);
return pathMatch ? pathMatch[1] : 'en';
const pathMatch = window.location.pathname.match(/^\/([a-z]{2})\//i)
return pathMatch ? pathMatch[1] : 'en'
}

async select(event) {
const locale = this.getDocumentLocale();
const { responseType, id, name, isDefaultOption } = event.target.dataset;
await trackUserAction("ApplicantSelectedEmployerOrPlatformItem", {
const locale = this.getDocumentLocale()
const { responseType, id, name, isDefaultOption } = event.target.dataset
await trackUserAction('ApplicantSelectedEmployerOrPlatformItem', {
item_type: responseType,
item_id: id,
item_name: name,
is_default_option: isDefaultOption,
locale
locale,
})

this.disableButtons()
const { token } = await fetchToken(responseType, id, locale);
this.submit(token);
const { token } = await fetchToken(responseType, id, locale)
this.submit(token)
}

submit(token) {
this.pinwheel.then(Pinwheel => initializePinwheel(Pinwheel, token, {
onEvent: this.onPinwheelEvent.bind(this),
onExit: this.reenableButtons.bind(this),
}));
this.pinwheel.then((Pinwheel) =>
initializePinwheel(Pinwheel, token, {
onEvent: this.onPinwheelEvent.bind(this),
onExit: this.reenableButtons.bind(this),
})
)
}

disableButtons() {
this.employerButtonTargets
.forEach(el => el.setAttribute("disabled", "disabled"))
this.employerButtonTargets.forEach((el) => el.setAttribute('disabled', 'disabled'))
}

reenableButtons() {
this.employerButtonTargets
.forEach(el => el.removeAttribute("disabled"))
this.employerButtonTargets.forEach((el) => el.removeAttribute('disabled'))
}

showHelpBanner() {
const url = new URL(window.location.href);
url.searchParams.set('help', 'true');
window.location.href = url.toString();
const url = new URL(window.location.href)
url.searchParams.set('help', 'true')
window.location.href = url.toString()
}
}
39 changes: 21 additions & 18 deletions app/app/javascript/controllers/cbv/synchronizations_controller.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,31 @@
import { Controller } from "@hotwired/stimulus"
import { Controller } from '@hotwired/stimulus'
import * as ActionCable from '@rails/actioncable'

export default class extends Controller {
static targets = ["form", "userAccountId"];
static targets = ['form', 'userAccountId']

cable = ActionCable.createConsumer();
cable = ActionCable.createConsumer()

connect() {
this.cable.subscriptions.create({ channel: 'PaystubsChannel', account_id: this.userAccountIdTarget.value }, {
connected: () => {
console.log("Connected to the channel:", this);
},
disconnected: () => {
console.log("Disconnected");
},
received: (data) => {
if (data.event === 'cbv.status_update') {
if (data.has_fully_synced) {
const accountId = data.account_id;
this.userAccountIdTarget.value = accountId;
this.formTarget.submit();
this.cable.subscriptions.create(
{ channel: 'PaystubsChannel', account_id: this.userAccountIdTarget.value },
{
connected: () => {
console.log('Connected to the channel:', this)
},
disconnected: () => {
console.log('Disconnected')
},
received: (data) => {
if (data.event === 'cbv.status_update') {
if (data.has_fully_synced) {
const accountId = data.account_id
this.userAccountIdTarget.value = accountId
this.formTarget.submit()
}
}
}
},
}
});
)
}
}
28 changes: 14 additions & 14 deletions app/app/javascript/controllers/help.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
import { Controller } from "@hotwired/stimulus"
import { trackUserAction } from "../utilities/help"
import { Controller } from '@hotwired/stimulus'
import { trackUserAction } from '../utilities/help'

export default class extends Controller {
static targets = ["iframe"]
static targets = ['iframe']

connect() {
this.handleClick = (event) => {
if (event.target.href?.includes("#help-modal")) {
trackUserAction("ApplicantOpenedHelpModal", event.target.dataset.source)
if (event.target.href?.includes('#help-modal')) {
trackUserAction('ApplicantOpenedHelpModal', event.target.dataset.source)
}
}
document.addEventListener("click", this.handleClick)

document.addEventListener('click', this.handleClick)
}

disconnect() {
document.removeEventListener("click", this.handleClick)
document.removeEventListener('click', this.handleClick)
}

/**
Expand All @@ -27,14 +27,14 @@ export default class extends Controller {
prepareNextUrl() {
const iframe = this.iframeTarget
const currentSrc = new URL(iframe.src)

// Generate a new random parameter
const randomHex = Array.from(crypto.getRandomValues(new Uint8Array(2)))
.map(b => b.toString(16).padStart(2, "0"))
.join("")
.map((b) => b.toString(16).padStart(2, '0'))
.join('')

// Update the iframe src with new random parameter
currentSrc.searchParams.set("r", randomHex)
currentSrc.searchParams.set('r', randomHex)
iframe.src = currentSrc.toString()
}
}
}
14 changes: 7 additions & 7 deletions app/app/javascript/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
// Run that command whenever you add a new controller or create them with
// ./bin/rails generate stimulus controllerName

import { application } from "./application"
import { application } from './application'

import CbvEmployerSearch from "./cbv/employer_search"
import CbvSynchronizationsController from "./cbv/synchronizations_controller"
import HelpController from "./help"
import CbvEmployerSearch from './cbv/employer_search'
import CbvSynchronizationsController from './cbv/synchronizations_controller'
import HelpController from './help'

application.register("cbv-employer-search", CbvEmployerSearch)
application.register("cbv-synchronizations", CbvSynchronizationsController)
application.register("help", HelpController)
application.register('cbv-employer-search', CbvEmployerSearch)
application.register('cbv-synchronizations', CbvSynchronizationsController)
application.register('help', HelpController)
Loading