Skip to content
This repository has been archived by the owner on Aug 26, 2022. It is now read-only.

Bug 1259173 - Adjust Akismet payload #3835

Merged
merged 1 commit into from Apr 20, 2016
Merged

Conversation

jwhitlock
Copy link
Contributor

@jwhitlock jwhitlock commented Apr 12, 2016

Rewrite the RevisionForm tests to setup forms for validation in the same way the views set them up (create, edit, and translate). Then:

Update the Akismet payload:

  • Add HTTP headers
  • Add blog parameter (front page of site), and fix default if omitted
  • Add permalink parameter (page that is being edited
  • Minimize comment_content to the changed and new content

Further improvements:

  • Move payload generation to new AkismetRevisionData class, so the same logic can be used in the RevisionForm as other places
  • Save Akismet data in RevisionIP instance, and replay for submit-ham and submit-spam.
  • Add read-only displays of Akismet data to admin detail views.

This PR should be manually merged:

  • The first commit rewrites the tests, and the PR will be easier to understand after merging (optional)
  • The second-to-last commit includes a database migration that should be applied in production before the last commit
  • The last commit adds the code that uses the new database column.

@@ -476,17 +478,21 @@ class RevisionAkismetSubmissionAdmin(DisabledDeletionMixin, admin.ModelAdmin):

def get_fields(self, request, obj=None):
if obj is None:
return ['type', 'revision']
self.revision_id = request.GET.get('revision')
Copy link
Contributor

Choose a reason for hiding this comment

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

admin instances are global, that will lead to threading issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AdminFormWithRequest makes more sense now. Is that the pattern to use here as well, or do you have a better solution?

Update - AdminFormWithRequest won't help here. Investigating InlineModelAdmin.

Update 2 - Nope, InlineModelAdmin is for editing to-many relations. Maybe a custom template?

@jwhitlock
Copy link
Contributor Author

Force push to change the strategy for displaying captured data from RevisionIP when creating a RevisionAkismetSubmission from the revision dashboard. Previously, the request query parameter was captured in get_fields, which was not thread safe. Now, add_view and change_view are adjusted to inject the data into the template context, and the template displays it (almost) like it was a field.

@jwhitlock jwhitlock force-pushed the akismet-payload-1259173 branch 3 times, most recently from 034b559 to 299d877 Compare April 19, 2016 20:24
@jwhitlock
Copy link
Contributor Author

Force push to rebase and renumber the migration.

jwhitlock added a commit that referenced this pull request Apr 19, 2016
Remaining parts of the PR need to wait until after the production push.
When Akismet identfies an edit as not spam, save the submission details
in case it has to be submitted as spam.
@jwhitlock jwhitlock merged commit 57728d3 into master Apr 20, 2016
@jwhitlock jwhitlock deleted the akismet-payload-1259173 branch April 20, 2016 15:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants