Fossil SCM
Refactor setting of forum post 'closed' tag so that it can be reused for other tags.
Commit
4d58d6e834dfd7bd9b5b79dfd7d24718419d1c5a3bd1ec72b379fc68f02c57f6
Parent
2d7e2cb24e2ebd7…
1 file changed
+42
-39
+42
-39
| --- src/forum.c | ||
| +++ src/forum.c | ||
| @@ -170,82 +170,84 @@ | ||
| 170 | 170 | } |
| 171 | 171 | return i ? -rc : rc; |
| 172 | 172 | } |
| 173 | 173 | |
| 174 | 174 | /* |
| 175 | -** Closes or re-opens the given forum RID via addition of a new | |
| 176 | -** control artifact into the repository. In order to provide | |
| 177 | -** consistent behavior for implied closing of responses and later | |
| 178 | -** versions, it always acts on the first version of the given forum | |
| 179 | -** post, walking the forumpost.fprev values to find the head of the | |
| 180 | -** chain. | |
| 175 | +** Applies or cancels a tag named zTagName on the given forum RID via | |
| 176 | +** addition of a new control artifact into the repository. In order to | |
| 177 | +** provide consistent behavior, it always acts on the first version of | |
| 178 | +** the given forum post, walking the forumpost.fprev values to find | |
| 179 | +** the head of the chain. | |
| 181 | 180 | ** |
| 182 | -** If doClose is true then a propagating "closed" tag is added, except | |
| 183 | -** as noted below, with the given optional zReason string as the tag's | |
| 184 | -** value. If doClose is false then any active "closed" tag on frid is | |
| 181 | +** If addTag is true then a propagating tag is added, except as noted | |
| 182 | +** below, with the given optional zReason string as the tag's | |
| 183 | +** value. If addTag is false then any matching active tag on frid is | |
| 185 | 184 | ** cancelled, except as noted below. zReason is ignored if doClose is |
| 186 | 185 | ** false or if zReason is NULL or starts with a NUL byte. |
| 187 | 186 | ** |
| 188 | -** This function only adds a "closed" tag if forum_rid_is_tagged() | |
| 189 | -** indicates that frid's head is not closed. If a parent post is | |
| 190 | -** already closed, no tag is added. Similarly, it will only remove a | |
| 191 | -** "closed" tag from a post which has its own "closed" tag, and will | |
| 192 | -** not remove an inherited one from a parent post. | |
| 187 | +** This function only adds a tag if forum_rid_is_tagged() indicates | |
| 188 | +** that frid's head is not tagged. If a parent post is already tagged, | |
| 189 | +** no tag is added. Similarly, it will only remove a tagtag from a | |
| 190 | +** post which has its own tag tag, and will not remove an inherited | |
| 191 | +** one from a parent post. | |
| 193 | 192 | ** |
| 194 | -** If doClose is true and frid is closed (directly or inherited), this | |
| 195 | -** is a no-op. Likewise, if doClose is false and frid itself is not | |
| 196 | -** closed (not accounting for an inherited closed tag), this is a | |
| 197 | -** no-op. | |
| 193 | +** If addTag is true and frid is already tagged (directly or | |
| 194 | +** inherited), this is a no-op. Likewise, if addTag is false and frid | |
| 195 | +** itself is not tagged (not accounting for an inherited closed tag), | |
| 196 | +** this is a no-op. | |
| 198 | 197 | ** |
| 199 | 198 | ** Returns true if it actually creates a new tag, else false. Fails |
| 200 | -** fatally on error. If it returns true then any ForumPost::iClosed | |
| 201 | -** values from previously loaded posts are invalidated if they refer | |
| 202 | -** to the amended post or a response to it. | |
| 199 | +** fatally on error. | |
| 200 | +** | |
| 201 | +** If it returns true then state from previously-loaded posts may be | |
| 202 | +** invalidated if they refer to the amended post or a response to it. | |
| 203 | +** e.g. if zTagName is "closed" then ForumPost::iClosed values may be | |
| 204 | +** stale. | |
| 203 | 205 | ** |
| 204 | 206 | ** Sidebars: |
| 205 | 207 | ** |
| 206 | 208 | ** - Unless the caller has a transaction open, via |
| 207 | 209 | ** db_begin_transaction(), there is a very tiny race condition |
| 208 | 210 | ** window during which the caller's idea of whether or not the forum |
| 209 | -** post is closed may differ from the current repository state. | |
| 211 | +** post is tagged may differ from the current repository state. | |
| 210 | 212 | ** |
| 211 | 213 | ** - This routine assumes that frid really does refer to a forum post. |
| 212 | 214 | ** |
| 213 | 215 | ** - This routine assumes that frid is not private or pending |
| 214 | 216 | ** moderation. |
| 215 | 217 | ** |
| 216 | -** - Closure of a forum post requires a propagating "closed" tag to | |
| 218 | +** - The applied tag is propagating so so that "closed" tags can | |
| 217 | 219 | ** account for how edits of posts are handled. This differs from |
| 218 | 220 | ** closure of a branch, where a non-propagating tag is used. |
| 219 | 221 | */ |
| 220 | -static int forumpost_close(int frid, int doClose, const char *zReason){ | |
| 222 | +static int forumpost_tag(int frid, const char *zTagName, int addTag, | |
| 223 | + const char *zReason){ | |
| 221 | 224 | Blob artifact = BLOB_INITIALIZER; /* Output artifact */ |
| 222 | 225 | Blob cksum = BLOB_INITIALIZER; /* Z-card */ |
| 223 | - int iClosed; /* true if frid is closed */ | |
| 226 | + int iTagged; /* true if frid is already tagged */ | |
| 224 | 227 | int trid; /* RID of new control artifact */ |
| 225 | 228 | char *zUuid; /* UUID of head version of post */ |
| 226 | 229 | |
| 227 | 230 | db_begin_transaction(); |
| 228 | 231 | frid = forumpost_head_rid(frid); |
| 229 | - iClosed = forum_rid_is_tagged(frid, "closed", 1); | |
| 230 | - if( (iClosed && doClose | |
| 231 | - /* Already closed, noting that in the case of (iClosed<0), it's | |
| 232 | - ** actually a parent which is closed. */) | |
| 233 | - || (iClosed<=0 && !doClose | |
| 234 | - /* This entry is not closed, but a parent post may be. */) ){ | |
| 232 | + iTagged = forum_rid_is_tagged(frid, "closed", 1); | |
| 233 | + if( (iTagged && addTag | |
| 234 | + /* Already tagged, noting that in the case of (addTag<0) it may | |
| 235 | + ** actually be a parent which is tagged. */) | |
| 236 | + || (iTagged<=0 && !addTag | |
| 237 | + /* This entry is not tagged, but a parent post may be. */) ){ | |
| 235 | 238 | db_end_transaction(0); |
| 236 | 239 | return 0; |
| 237 | 240 | } |
| 238 | - if( doClose==0 || (zReason && !zReason[0]) ){ | |
| 241 | + if( addTag==0 || (zReason && !zReason[0]) ){ | |
| 239 | 242 | zReason = 0; |
| 240 | 243 | } |
| 241 | 244 | zUuid = rid_to_uuid(frid); |
| 242 | 245 | blob_appendf(&artifact, "D %z\n", date_in_standard_format( "now" )); |
| 243 | - blob_appendf(&artifact, | |
| 244 | - "T %cclosed %s%s%F\n", | |
| 245 | - doClose ? '*' : '-', zUuid, | |
| 246 | - zReason ? " " : "", zReason ? zReason : ""); | |
| 246 | + blob_appendf(&artifact, "T %c%s %s%s%F\n", | |
| 247 | + addTag ? '*' : '-', zTagName, | |
| 248 | + zUuid, zReason ? " " : "", zReason ? zReason : ""); | |
| 247 | 249 | blob_appendf(&artifact, "U %F\n", login_name()); |
| 248 | 250 | md5sum_blob(&artifact, &cksum); |
| 249 | 251 | blob_appendf(&artifact, "Z %b\n", &cksum); |
| 250 | 252 | blob_reset(&cksum); |
| 251 | 253 | trid = content_put_ex(&artifact, 0, 0, 0, 0); |
| @@ -256,11 +258,12 @@ | ||
| 256 | 258 | MC_NONE /*MC_PERMIT_HOOKS?*/)==0 ){ |
| 257 | 259 | fossil_fatal("%s", g.zErrMsg); |
| 258 | 260 | } |
| 259 | 261 | assert( blob_is_reset(&artifact) ); |
| 260 | 262 | db_add_unsent(trid); |
| 261 | - admin_log("%s forum post %S", doClose ? "Close" : "Re-open", zUuid); | |
| 263 | + admin_log("Tag forum post %S with %c%s", | |
| 264 | + zUuid, addTag ? '*' : '-', zTagName); | |
| 262 | 265 | fossil_free(zUuid); |
| 263 | 266 | /* Potential TODO: if (iClosed>0) then we could find the initial tag |
| 264 | 267 | ** artifact and content_deltify(thatRid,&trid,1,0). Given the tiny |
| 265 | 268 | ** size of these artifacts, however, that would save little space, |
| 266 | 269 | ** if any. */ |
| @@ -1447,11 +1450,11 @@ | ||
| 1447 | 1450 | ** |
| 1448 | 1451 | ** fpid=X Hash of the post to be edited. REQUIRED |
| 1449 | 1452 | ** reason=X Optional reason for closure. |
| 1450 | 1453 | ** |
| 1451 | 1454 | ** Closes or re-opens the given forum post, within the bounds of the |
| 1452 | -** API for forumpost_close(). After (perhaps) modifying the "closed" | |
| 1455 | +** API for forumpost_tag(). After (perhaps) modifying the "closed" | |
| 1453 | 1456 | ** status of the given thread, it redirects to that post's thread |
| 1454 | 1457 | ** view. Requires admin privileges. |
| 1455 | 1458 | */ |
| 1456 | 1459 | void forum_page_close(void){ |
| 1457 | 1460 | const char *zFpid = PD("fpid",""); |
| @@ -1469,11 +1472,11 @@ | ||
| 1469 | 1472 | if( fpid<=0 ){ |
| 1470 | 1473 | webpage_error("Missing or invalid fpid query parameter"); |
| 1471 | 1474 | } |
| 1472 | 1475 | fClose = sqlite3_strglob("*_close*", g.zPath)==0; |
| 1473 | 1476 | if( fClose ) zReason = PD("reason",0); |
| 1474 | - forumpost_close(fpid, fClose, zReason); | |
| 1477 | + forumpost_tag(fpid, "closed", fClose, zReason); | |
| 1475 | 1478 | cgi_redirectf("%R/forumpost/%S",zFpid); |
| 1476 | 1479 | return; |
| 1477 | 1480 | } |
| 1478 | 1481 | |
| 1479 | 1482 | /* |
| 1480 | 1483 |
| --- src/forum.c | |
| +++ src/forum.c | |
| @@ -170,82 +170,84 @@ | |
| 170 | } |
| 171 | return i ? -rc : rc; |
| 172 | } |
| 173 | |
| 174 | /* |
| 175 | ** Closes or re-opens the given forum RID via addition of a new |
| 176 | ** control artifact into the repository. In order to provide |
| 177 | ** consistent behavior for implied closing of responses and later |
| 178 | ** versions, it always acts on the first version of the given forum |
| 179 | ** post, walking the forumpost.fprev values to find the head of the |
| 180 | ** chain. |
| 181 | ** |
| 182 | ** If doClose is true then a propagating "closed" tag is added, except |
| 183 | ** as noted below, with the given optional zReason string as the tag's |
| 184 | ** value. If doClose is false then any active "closed" tag on frid is |
| 185 | ** cancelled, except as noted below. zReason is ignored if doClose is |
| 186 | ** false or if zReason is NULL or starts with a NUL byte. |
| 187 | ** |
| 188 | ** This function only adds a "closed" tag if forum_rid_is_tagged() |
| 189 | ** indicates that frid's head is not closed. If a parent post is |
| 190 | ** already closed, no tag is added. Similarly, it will only remove a |
| 191 | ** "closed" tag from a post which has its own "closed" tag, and will |
| 192 | ** not remove an inherited one from a parent post. |
| 193 | ** |
| 194 | ** If doClose is true and frid is closed (directly or inherited), this |
| 195 | ** is a no-op. Likewise, if doClose is false and frid itself is not |
| 196 | ** closed (not accounting for an inherited closed tag), this is a |
| 197 | ** no-op. |
| 198 | ** |
| 199 | ** Returns true if it actually creates a new tag, else false. Fails |
| 200 | ** fatally on error. If it returns true then any ForumPost::iClosed |
| 201 | ** values from previously loaded posts are invalidated if they refer |
| 202 | ** to the amended post or a response to it. |
| 203 | ** |
| 204 | ** Sidebars: |
| 205 | ** |
| 206 | ** - Unless the caller has a transaction open, via |
| 207 | ** db_begin_transaction(), there is a very tiny race condition |
| 208 | ** window during which the caller's idea of whether or not the forum |
| 209 | ** post is closed may differ from the current repository state. |
| 210 | ** |
| 211 | ** - This routine assumes that frid really does refer to a forum post. |
| 212 | ** |
| 213 | ** - This routine assumes that frid is not private or pending |
| 214 | ** moderation. |
| 215 | ** |
| 216 | ** - Closure of a forum post requires a propagating "closed" tag to |
| 217 | ** account for how edits of posts are handled. This differs from |
| 218 | ** closure of a branch, where a non-propagating tag is used. |
| 219 | */ |
| 220 | static int forumpost_close(int frid, int doClose, const char *zReason){ |
| 221 | Blob artifact = BLOB_INITIALIZER; /* Output artifact */ |
| 222 | Blob cksum = BLOB_INITIALIZER; /* Z-card */ |
| 223 | int iClosed; /* true if frid is closed */ |
| 224 | int trid; /* RID of new control artifact */ |
| 225 | char *zUuid; /* UUID of head version of post */ |
| 226 | |
| 227 | db_begin_transaction(); |
| 228 | frid = forumpost_head_rid(frid); |
| 229 | iClosed = forum_rid_is_tagged(frid, "closed", 1); |
| 230 | if( (iClosed && doClose |
| 231 | /* Already closed, noting that in the case of (iClosed<0), it's |
| 232 | ** actually a parent which is closed. */) |
| 233 | || (iClosed<=0 && !doClose |
| 234 | /* This entry is not closed, but a parent post may be. */) ){ |
| 235 | db_end_transaction(0); |
| 236 | return 0; |
| 237 | } |
| 238 | if( doClose==0 || (zReason && !zReason[0]) ){ |
| 239 | zReason = 0; |
| 240 | } |
| 241 | zUuid = rid_to_uuid(frid); |
| 242 | blob_appendf(&artifact, "D %z\n", date_in_standard_format( "now" )); |
| 243 | blob_appendf(&artifact, |
| 244 | "T %cclosed %s%s%F\n", |
| 245 | doClose ? '*' : '-', zUuid, |
| 246 | zReason ? " " : "", zReason ? zReason : ""); |
| 247 | blob_appendf(&artifact, "U %F\n", login_name()); |
| 248 | md5sum_blob(&artifact, &cksum); |
| 249 | blob_appendf(&artifact, "Z %b\n", &cksum); |
| 250 | blob_reset(&cksum); |
| 251 | trid = content_put_ex(&artifact, 0, 0, 0, 0); |
| @@ -256,11 +258,12 @@ | |
| 256 | MC_NONE /*MC_PERMIT_HOOKS?*/)==0 ){ |
| 257 | fossil_fatal("%s", g.zErrMsg); |
| 258 | } |
| 259 | assert( blob_is_reset(&artifact) ); |
| 260 | db_add_unsent(trid); |
| 261 | admin_log("%s forum post %S", doClose ? "Close" : "Re-open", zUuid); |
| 262 | fossil_free(zUuid); |
| 263 | /* Potential TODO: if (iClosed>0) then we could find the initial tag |
| 264 | ** artifact and content_deltify(thatRid,&trid,1,0). Given the tiny |
| 265 | ** size of these artifacts, however, that would save little space, |
| 266 | ** if any. */ |
| @@ -1447,11 +1450,11 @@ | |
| 1447 | ** |
| 1448 | ** fpid=X Hash of the post to be edited. REQUIRED |
| 1449 | ** reason=X Optional reason for closure. |
| 1450 | ** |
| 1451 | ** Closes or re-opens the given forum post, within the bounds of the |
| 1452 | ** API for forumpost_close(). After (perhaps) modifying the "closed" |
| 1453 | ** status of the given thread, it redirects to that post's thread |
| 1454 | ** view. Requires admin privileges. |
| 1455 | */ |
| 1456 | void forum_page_close(void){ |
| 1457 | const char *zFpid = PD("fpid",""); |
| @@ -1469,11 +1472,11 @@ | |
| 1469 | if( fpid<=0 ){ |
| 1470 | webpage_error("Missing or invalid fpid query parameter"); |
| 1471 | } |
| 1472 | fClose = sqlite3_strglob("*_close*", g.zPath)==0; |
| 1473 | if( fClose ) zReason = PD("reason",0); |
| 1474 | forumpost_close(fpid, fClose, zReason); |
| 1475 | cgi_redirectf("%R/forumpost/%S",zFpid); |
| 1476 | return; |
| 1477 | } |
| 1478 | |
| 1479 | /* |
| 1480 |
| --- src/forum.c | |
| +++ src/forum.c | |
| @@ -170,82 +170,84 @@ | |
| 170 | } |
| 171 | return i ? -rc : rc; |
| 172 | } |
| 173 | |
| 174 | /* |
| 175 | ** Applies or cancels a tag named zTagName on the given forum RID via |
| 176 | ** addition of a new control artifact into the repository. In order to |
| 177 | ** provide consistent behavior, it always acts on the first version of |
| 178 | ** the given forum post, walking the forumpost.fprev values to find |
| 179 | ** the head of the chain. |
| 180 | ** |
| 181 | ** If addTag is true then a propagating tag is added, except as noted |
| 182 | ** below, with the given optional zReason string as the tag's |
| 183 | ** value. If addTag is false then any matching active tag on frid is |
| 184 | ** cancelled, except as noted below. zReason is ignored if doClose is |
| 185 | ** false or if zReason is NULL or starts with a NUL byte. |
| 186 | ** |
| 187 | ** This function only adds a tag if forum_rid_is_tagged() indicates |
| 188 | ** that frid's head is not tagged. If a parent post is already tagged, |
| 189 | ** no tag is added. Similarly, it will only remove a tagtag from a |
| 190 | ** post which has its own tag tag, and will not remove an inherited |
| 191 | ** one from a parent post. |
| 192 | ** |
| 193 | ** If addTag is true and frid is already tagged (directly or |
| 194 | ** inherited), this is a no-op. Likewise, if addTag is false and frid |
| 195 | ** itself is not tagged (not accounting for an inherited closed tag), |
| 196 | ** this is a no-op. |
| 197 | ** |
| 198 | ** Returns true if it actually creates a new tag, else false. Fails |
| 199 | ** fatally on error. |
| 200 | ** |
| 201 | ** If it returns true then state from previously-loaded posts may be |
| 202 | ** invalidated if they refer to the amended post or a response to it. |
| 203 | ** e.g. if zTagName is "closed" then ForumPost::iClosed values may be |
| 204 | ** stale. |
| 205 | ** |
| 206 | ** Sidebars: |
| 207 | ** |
| 208 | ** - Unless the caller has a transaction open, via |
| 209 | ** db_begin_transaction(), there is a very tiny race condition |
| 210 | ** window during which the caller's idea of whether or not the forum |
| 211 | ** post is tagged may differ from the current repository state. |
| 212 | ** |
| 213 | ** - This routine assumes that frid really does refer to a forum post. |
| 214 | ** |
| 215 | ** - This routine assumes that frid is not private or pending |
| 216 | ** moderation. |
| 217 | ** |
| 218 | ** - The applied tag is propagating so so that "closed" tags can |
| 219 | ** account for how edits of posts are handled. This differs from |
| 220 | ** closure of a branch, where a non-propagating tag is used. |
| 221 | */ |
| 222 | static int forumpost_tag(int frid, const char *zTagName, int addTag, |
| 223 | const char *zReason){ |
| 224 | Blob artifact = BLOB_INITIALIZER; /* Output artifact */ |
| 225 | Blob cksum = BLOB_INITIALIZER; /* Z-card */ |
| 226 | int iTagged; /* true if frid is already tagged */ |
| 227 | int trid; /* RID of new control artifact */ |
| 228 | char *zUuid; /* UUID of head version of post */ |
| 229 | |
| 230 | db_begin_transaction(); |
| 231 | frid = forumpost_head_rid(frid); |
| 232 | iTagged = forum_rid_is_tagged(frid, "closed", 1); |
| 233 | if( (iTagged && addTag |
| 234 | /* Already tagged, noting that in the case of (addTag<0) it may |
| 235 | ** actually be a parent which is tagged. */) |
| 236 | || (iTagged<=0 && !addTag |
| 237 | /* This entry is not tagged, but a parent post may be. */) ){ |
| 238 | db_end_transaction(0); |
| 239 | return 0; |
| 240 | } |
| 241 | if( addTag==0 || (zReason && !zReason[0]) ){ |
| 242 | zReason = 0; |
| 243 | } |
| 244 | zUuid = rid_to_uuid(frid); |
| 245 | blob_appendf(&artifact, "D %z\n", date_in_standard_format( "now" )); |
| 246 | blob_appendf(&artifact, "T %c%s %s%s%F\n", |
| 247 | addTag ? '*' : '-', zTagName, |
| 248 | zUuid, zReason ? " " : "", zReason ? zReason : ""); |
| 249 | blob_appendf(&artifact, "U %F\n", login_name()); |
| 250 | md5sum_blob(&artifact, &cksum); |
| 251 | blob_appendf(&artifact, "Z %b\n", &cksum); |
| 252 | blob_reset(&cksum); |
| 253 | trid = content_put_ex(&artifact, 0, 0, 0, 0); |
| @@ -256,11 +258,12 @@ | |
| 258 | MC_NONE /*MC_PERMIT_HOOKS?*/)==0 ){ |
| 259 | fossil_fatal("%s", g.zErrMsg); |
| 260 | } |
| 261 | assert( blob_is_reset(&artifact) ); |
| 262 | db_add_unsent(trid); |
| 263 | admin_log("Tag forum post %S with %c%s", |
| 264 | zUuid, addTag ? '*' : '-', zTagName); |
| 265 | fossil_free(zUuid); |
| 266 | /* Potential TODO: if (iClosed>0) then we could find the initial tag |
| 267 | ** artifact and content_deltify(thatRid,&trid,1,0). Given the tiny |
| 268 | ** size of these artifacts, however, that would save little space, |
| 269 | ** if any. */ |
| @@ -1447,11 +1450,11 @@ | |
| 1450 | ** |
| 1451 | ** fpid=X Hash of the post to be edited. REQUIRED |
| 1452 | ** reason=X Optional reason for closure. |
| 1453 | ** |
| 1454 | ** Closes or re-opens the given forum post, within the bounds of the |
| 1455 | ** API for forumpost_tag(). After (perhaps) modifying the "closed" |
| 1456 | ** status of the given thread, it redirects to that post's thread |
| 1457 | ** view. Requires admin privileges. |
| 1458 | */ |
| 1459 | void forum_page_close(void){ |
| 1460 | const char *zFpid = PD("fpid",""); |
| @@ -1469,11 +1472,11 @@ | |
| 1472 | if( fpid<=0 ){ |
| 1473 | webpage_error("Missing or invalid fpid query parameter"); |
| 1474 | } |
| 1475 | fClose = sqlite3_strglob("*_close*", g.zPath)==0; |
| 1476 | if( fClose ) zReason = PD("reason",0); |
| 1477 | forumpost_tag(fpid, "closed", fClose, zReason); |
| 1478 | cgi_redirectf("%R/forumpost/%S",zFpid); |
| 1479 | return; |
| 1480 | } |
| 1481 | |
| 1482 | /* |
| 1483 |