Skip to content
This repository was archived by the owner on Apr 21, 2026. It is now read-only.

Commit 3270cfb

Browse files
bpamiriclaude
bpamiri
and
claude
committed
refactor: Extract duplicate code, fix N+1 queries, validate bulk ops, standardize isObject
- Extract duplicate getContributors() from HomeController and CommunityController into base Controller.cfc (with cache=60 on DB queries) - Fix 6 N+1 query problems in admin views by pre-fetching lookup maps in controllers (blog categories, parent comments, parent categories, contributor roles) - Add input validation guard clauses to blogBulkApprove() and blogBulkReject() to handle missing/empty selectedBlogIds gracefully - Standardize model object null-checking from isNull() to isObject() across 12 controller files (~24 locations), keeping isNull() only for genuine null checks on variable values and query columns - Fix variable shadowing bug in contributors.cfm (inner loop reused outer index) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e78e089 commit 3270cfb

21 files changed

Lines changed: 247 additions & 284 deletions

‎app/controllers/Controller.cfc‎

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,115 @@ component extends="wheels.Controller" {
125125
return model("Attachment").findAllByBlogId(value="#arguments.id#", cache=10);
126126
}
127127

128+
public function getContributors(){
129+
var contributorsList = model("contributors").findAll(where="username!='dependabot[bot]'", cache=60);
130+
131+
if(contributorsList.recordCount == 0){
132+
return [];
133+
}
134+
135+
if(dateDiff("d", contributorsList.updatedAt, now()) >= 30){
136+
var contributorLink = model("setting").findAll(select="wheelsContributorLink");
137+
if(contributorLink.recordCount == 0 OR contributorLink.wheelsContributorLink == ""){
138+
return [];
139+
}
140+
var apiUrl = contributorLink.wheelsContributorLink;
141+
142+
// GitHub requires a User-Agent header
143+
cfhttp(url="#apiUrl#", method="get", timeout=30, result="resp"){
144+
cfhttpparam(type="header", name="User-Agent", value="CFWheels-App");
145+
}
146+
if (listFirst(resp.statusCode, " ") EQ "200") {
147+
contributors = deserializeJson(resp.fileContent);
148+
149+
// loop contributors and enrich with real name
150+
for (i=1; i <= arrayLen(contributors); i++) {
151+
username = contributors[i].login;
152+
userUrl = "https://api.github.com/users/" & username;
153+
154+
cfhttp(url=userUrl, method="get", timeout=30, result="userResp") {
155+
cfhttpparam(type="header", name="User-Agent", value="CFWheels-App");
156+
}
157+
158+
if (listFirst(userResp.statusCode, " ") EQ "200") {
159+
userData = deserializeJson(userResp.fileContent);
160+
161+
// GitHub may return null if no name is set
162+
if (structKeyExists(userData, "name") AND len(trim(userData.name))) {
163+
contributors[i].name = userData.name;
164+
} else {
165+
contributors[i].name = contributors[i].login; // fallback if not set
166+
}
167+
168+
// === Insert only if not exists ===
169+
var existing = model("Contributor").findOneByUsername(username);
170+
171+
if (!isObject(existing)) {
172+
var c = model("Contributor").new();
173+
c.name = contributors[i].name;
174+
c.username = contributors[i].login;
175+
c.description = "";
176+
c.roles = "1";
177+
c.profilePic = userData.avatar_url;
178+
c.contributions = contributors[i].contributions;
179+
c.profileAPI = userData.url;
180+
c.LinkedInLink = "";
181+
c.save();
182+
}else{
183+
var c = existing;
184+
c.name = contributors[i].name;
185+
c.username = contributors[i].login;
186+
c.profilePic = userData.avatar_url;
187+
c.contributions = contributors[i].contributions;
188+
c.profileAPI = userData.url;
189+
c.save();
190+
}
191+
}
192+
}
193+
contributors = model("contributors").findAll(where="username!='dependabot[bot]'");
194+
}else{
195+
contributors = contributorsList;
196+
}
197+
}else{
198+
contributors = contributorsList;
199+
}
200+
contributorsArray = [];
201+
// Preload all roles to avoid N+1 queries
202+
var rolesMap = {};
203+
for (r in model("contributor_role").findAll(cache=60)) {
204+
rolesMap[r.id] = r.rolename;
205+
}
206+
207+
for (row in contributors) {
208+
var roleString = "";
209+
var roleIds = (!isNull(row.roles) AND len(trim(row.roles))) ? listToArray(row.roles, ",") : [];
210+
211+
for (var j=1; j <= arrayLen(roleIds); j++) {
212+
var roleName = rolesMap[ roleIds[j] ] ?: ""; // safe lookup
213+
214+
if (len(roleName)) {
215+
if (j == 1) {
216+
roleString &= roleName;
217+
} else if (j == arrayLen(roleIds)) {
218+
roleString &= " and " & roleName;
219+
} else {
220+
roleString &= ", " & roleName;
221+
}
222+
}
223+
}
224+
225+
contributor = {
226+
"name" : row.name,
227+
"description" : row.description,
228+
"role" : roleString,
229+
"profilePic" : row.contributor_pic,
230+
"LinkedInLink": row.LinkedIn_Link
231+
};
232+
arrayAppend(contributorsArray, contributor);
233+
}
234+
return contributorsArray;
235+
}
236+
128237
function saveRedirectUrl(url) {
129238
session.redirectAfterLogin = arguments.url;
130239
}
@@ -380,7 +489,7 @@ component extends="wheels.Controller" {
380489
// Find the blog by ID
381490
var blog = model("Blog").findByKey(blogId);
382491

383-
if (isNull(blog)) {
492+
if (!isObject(blog)) {
384493
response.message = "Blog post not found for updating.";
385494
return response;
386495
}

‎app/controllers/admin/AdminController.cfc‎

Lines changed: 69 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ component extends="app.Controllers.Controller" {
1414

1515
function blog() {
1616
blogs = getAllBlogs();
17-
17+
blogCategoryMap = buildBlogCategoryMap(blogs);
1818
}
1919

2020
// Function to edit a blog post
@@ -159,16 +159,24 @@ component extends="app.Controllers.Controller" {
159159
select="id,Content,isPublished,commentParentId,createdat,authorId,blogId,slug,FullName,title",
160160
include="User, Blog",
161161
order="createdAt DESC");
162-
}
162+
parentCommentMap = buildParentCommentMap(comments);
163+
}
163164

164165
function viewComments(){
165166
id = params.id;
166167
comments = model("comment").findbyKey( key="#id#",
167168
select="id,Content,isPublished,commentParentId,createdat,authorId,blogId,slug,FullName,title",
168169
include="User, Blog"
169170
);
171+
parentCommentContent = "";
172+
if (isObject(comments) && val(comments.commentParentId) > 0) {
173+
var parentComment = model("comment").findByKey(key=val(comments.commentParentId), select="content");
174+
if (isObject(parentComment)) {
175+
parentCommentContent = parentComment.content;
176+
}
177+
}
170178
renderPartial(partial="partials/commentView");
171-
}
179+
}
172180

173181
function blogApprove() {
174182
try {
@@ -195,12 +203,19 @@ component extends="app.Controllers.Controller" {
195203
}
196204

197205
function blogBulkApprove(){
206+
if (!structKeyExists(params, "selectedBlogIds") || !isArray(params.selectedBlogIds) || arrayLen(params.selectedBlogIds) == 0) {
207+
blogs = getAllBlogs();
208+
blogCategoryMap = buildBlogCategoryMap(blogs);
209+
renderPartial(partial="partials/blogs");
210+
return;
211+
}
198212
try{
199213
for (blogId in params.selectedBlogIds) {
200214
var message = blogApproval(blogId);
201215
}
202216
success="Blogs are approved successfully!";
203217
blogs = getAllBlogs();
218+
blogCategoryMap = buildBlogCategoryMap(blogs);
204219
renderPartial(partial="partials/blogs");
205220
} catch(e) {
206221
cfheader(statusCode=500);
@@ -227,12 +242,19 @@ component extends="app.Controllers.Controller" {
227242
}
228243

229244
function blogBulkReject(){
245+
if (!structKeyExists(params, "selectedBlogIds") || !isArray(params.selectedBlogIds) || arrayLen(params.selectedBlogIds) == 0) {
246+
blogs = getAllBlogs();
247+
blogCategoryMap = buildBlogCategoryMap(blogs);
248+
renderPartial(partial="partials/blogs");
249+
return;
250+
}
230251
try{
231252
for (blogId in params.selectedBlogIds) {
232253
var message = blogReject(blogId);
233254
}
234255
success="Blogs are rejected successfully!";
235256
blogs = getAllBlogs();
257+
blogCategoryMap = buildBlogCategoryMap(blogs);
236258
renderPartial(partial="partials/blogs");
237259
} catch(e) {
238260
cfheader(statusCode=500);
@@ -281,7 +303,7 @@ component extends="app.Controllers.Controller" {
281303
var blog = model("Blog").findByKey(blogData.id);
282304
var user = model("user").findByKey(blog.createdby);
283305

284-
if (!isNull(blog)) {
306+
if (isObject(blog)) {
285307
if(blog.status == "Approved"){
286308
if(len(trim(publishDate))){
287309
blog.publishedAt = parseDateTime(publishDate);
@@ -346,7 +368,7 @@ component extends="app.Controllers.Controller" {
346368
function unpublishComment(){
347369
try{
348370
var comment = model("comment").findbyKey(key="#params.id#");
349-
if(!isNull(comment)){
371+
if(isObject(comment)){
350372
comment.isPublished = false;
351373
if(comment.save()){
352374
renderText('<span class="badge bg-danger">Hidden</span>');
@@ -548,6 +570,43 @@ component extends="app.Controllers.Controller" {
548570
: (structKeyExists(form, name) ? form[name] : defaultValue);
549571
}
550572

573+
private struct function buildBlogCategoryMap(required query blogs) {
574+
var map = {};
575+
var blogIds = valueList(arguments.blogs.id);
576+
if (!len(blogIds)) return map;
577+
var allCategories = model("BlogCategory").findAll(
578+
select="blogId,name",
579+
where="blogId IN (#blogIds#)",
580+
include="Blog,Category"
581+
);
582+
for (var row in allCategories) {
583+
if (!structKeyExists(map, row.blogId)) {
584+
map[row.blogId] = [];
585+
}
586+
arrayAppend(map[row.blogId], row.name);
587+
}
588+
return map;
589+
}
590+
591+
private struct function buildParentCommentMap(required query comments) {
592+
var map = {};
593+
var parentIds = [];
594+
for (var row in arguments.comments) {
595+
if (val(row.commentParentId) > 0) {
596+
arrayAppend(parentIds, val(row.commentParentId));
597+
}
598+
}
599+
if (!arrayLen(parentIds)) return map;
600+
var parentComments = model("comment").findAll(
601+
select="id,content",
602+
where="id IN (#arrayToList(parentIds)#)"
603+
);
604+
for (var pc in parentComments) {
605+
map[pc.id] = pc.content;
606+
}
607+
return map;
608+
}
609+
551610
public function getAllBlogs() {
552611
return model("Blog").findAll(
553612
include="User, PostStatus, PostType",
@@ -575,7 +634,7 @@ component extends="app.Controllers.Controller" {
575634
function publishComment(id){
576635
var comment = model("comment").findbyKey(key="#id#", include="Blog");
577636
var user = model("user").findByKey(comment.authorId);
578-
if(!isNull(comment)){
637+
if(isObject(comment)){
579638
comment.isPublished = true;
580639
if(comment.save()){
581640
siteurl = urlFor(route="blog-detail",slug=comment.blog.slug ,onlyPath=false);
@@ -616,8 +675,8 @@ component extends="app.Controllers.Controller" {
616675
var blog = model("Blog").findByKey(id);
617676
var user = model("user").findByKey(blog.createdby);
618677

619-
if (!isNull(blog)) {
620-
678+
if (isObject(blog)) {
679+
621680
blog.status = "Approved"; //approved
622681
if (blog.save()) {
623682
return {
@@ -643,8 +702,8 @@ component extends="app.Controllers.Controller" {
643702
var blog = model("Blog").findByKey(id);
644703
var user = model("user").findByKey(blog.createdby);
645704

646-
if (!isNull(blog)) {
647-
705+
if (isObject(blog)) {
706+
648707
blog.status = "Rejected"; //reject
649708
blog.publishedAt = "";
650709
if (blog.save()) {

‎app/controllers/admin/CategoriesController.cfc‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@ component extends="app.Controllers.Controller" {
1010

1111
function index(){
1212
categories = model("category").getAll();
13+
parentCategoryMap = buildParentCategoryMap(categories);
14+
}
15+
16+
private struct function buildParentCategoryMap(required query categories) {
17+
var map = {};
18+
var parentIds = [];
19+
for (var row in arguments.categories) {
20+
if (val(row.parentId) > 0) {
21+
arrayAppend(parentIds, val(row.parentId));
22+
}
23+
}
24+
if (!arrayLen(parentIds)) return map;
25+
var parents = model("category").findAll(
26+
select="id,name",
27+
where="id IN (#arrayToList(parentIds)#)"
28+
);
29+
for (var p in parents) {
30+
map[p.id] = p.name;
31+
}
32+
return map;
1333
}
1434

1535
function add(){
@@ -55,7 +75,7 @@ component extends="app.Controllers.Controller" {
5575
if (categoryData.id > 0) {
5676
var category = model("category").findByKey(categoryData.id);
5777

58-
if (not isNull(category)) {
78+
if (isObject(category)) {
5979
// Edit the existing category post
6080
category.name = categoryData.Name;
6181
category.description = categoryData.description;

‎app/controllers/admin/EmailTemplatesController.cfc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ component extends="app.Controllers.Controller" {
2424
try{
2525
email = model("emailTemplate").findOne(where="id = #val(params.id)#");
2626
if(structKeyExists(params, "id")){
27-
if (not isNull(email)) {
27+
if (isObject(email)) {
2828
// Edit the existing email post
2929
email.subject = params.subject;
3030
email.message = params.message;

‎app/controllers/admin/FeatureController.cfc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ component extends="app.Controllers.Controller" {
5858
var message = "";
5959
if (featureData.id > 0) {
6060
var feature = model("Feature").findByKey(featureData.id);
61-
if (!isNull(feature)) {
61+
if (isObject(feature)) {
6262
feature.title = featureData.title;
6363
feature.image = featureData.image;
6464
feature.description = featureData.description;
@@ -86,7 +86,7 @@ component extends="app.Controllers.Controller" {
8686
// Soft delete feature
8787
private function softDelete(id) {
8888
var feature = model("Feature").findByKey(arguments.id);
89-
if (!isNull(feature)) {
89+
if (isObject(feature)) {
9090
feature.deletedAt = now();
9191
feature.save();
9292
return { success = true, message = "Feature deleted successfully." };

‎app/controllers/admin/NewsletterController.cfc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ component extends="app.Controllers.Controller" {
203203

204204
if (type == "user") {
205205
var user = model("User").findOne(where="email = '#email#'");
206-
if (!isNull(user)) {
206+
if (isObject(user)) {
207207
user.update(newsletter=false);
208208
model("Log").log(
209209
category = "wheels.newsletter",
@@ -228,7 +228,7 @@ component extends="app.Controllers.Controller" {
228228
}
229229
} else {
230230
var subscriber = model("NewsletterSubscriber").findOne(where="email = '#email#'");
231-
if (!isNull(subscriber)) {
231+
if (isObject(subscriber)) {
232232
subscriber.update(status="inactive");
233233
model("Log").log(
234234
category = "wheels.newsletter",

‎app/controllers/admin/RolesController.cfc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ component extends="app.Controllers.Controller" {
7676
try{
7777
if (RoleData.id > 0) {
7878
var Role = model("Role").findByKey(RoleData.id);
79-
if (!isNull(Role)) {
79+
if (isObject(Role)) {
8080
// Edit the existing Role post
8181
Role.name = RoleData.Name;
8282
Role.description = RoleData.description;

0 commit comments

Comments
 (0)