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

Add Pronouns Support to Author Metadata and User Profiles #1695

Merged
merged 16 commits into from
Nov 27, 2024
Merged

Conversation

rabble
Copy link
Contributor

@rabble rabble commented Nov 20, 2024

Description

This pull request adds support for managing and displaying pronouns in the Author metadata and user profiles. The changes introduce the pronouns attribute to the Author Core Data entity, ensure it is properly hydrated from metadata events, and allow users to edit and publish their pronouns through the ProfileEditView. Pronouns are now fully integrated across the data model, user interface, and backend services.


Key Changes

  1. Core Data Updates:

    • Added a new pronouns attribute to the Author Core Data entity.
    • Updated the Core Data model to Nos 20 to support the new pronouns attribute.
    • Regenerated Author+CoreDataProperties to reflect the new attribute.
  2. Metadata Handling:

    • Extended MetadataEventJSON to include a pronouns property, allowing parsing and serialization of pronouns in metadata events.
    • Updated the hydrateMetaData function in Event+Hydration.swift to set Author.pronouns from parsed metadata.
  3. Profile Management:

    • Added a NosTextField for pronouns in ProfileEditView, allowing users to view and update their pronouns.
    • Synchronized author.pronouns with the backend through CurrentUser+PublishEvents.swift.
  4. Localization:

    • Added a new localized string for "Pronouns" in the Localizable.xcstrings file to support internationalization.
  5. Data Model Validation:

    • Updated the Author.dictionary property to include pronouns when generating metadata.

Outstanding Work

This PR requires database migration to Nos 20 before it can be safely merged. While the PR includes the necessary changes to the Core Data model, a migration path needs to be defined to ensure that the existing database can upgrade smoothly to the new schema.

Help Needed:
  • Define Migration Strategy:
    • Implement a lightweight migration for the Nos 20 version to include the new pronouns field in the Author entity.
    • Verify that the migration logic in the PersistenceController handles the new schema without data loss.
  • Update PersistenceController:
    • Increment the Core Data version in the PersistenceController from version = 3 to version = 20 to trigger the migration process.
Next Steps for Migration:
  1. Increment Database Version:
    Update the version in the PersistenceController:
    private static let version = 20
  2. Enable Automatic Lightweight Migration:
    Ensure the loadPersistentStores method includes migration options:
    container.persistentStoreDescriptions.first?.setOption(true as NSNumber, forKey: NSInferMappingModelAutomaticallyOption)
    container.persistentStoreDescriptions.first?.setOption(true as NSNumber, forKey: NSMigratePersistentStoresAutomaticallyOption)
  3. Test Migration:
    • Run the app on a pre-existing database (from Nos 19) to ensure the migration completes successfully.
    • Test data integrity after the migration.

Testing Steps

  1. Profile Edit View:

    • Navigate to the profile edit screen.
    • Verify that a "Pronouns" text field appears and updates correctly.
    • Save changes and confirm the pronouns are displayed on the user profile.
  2. Metadata Hydration:

    • Parse metadata events containing pronouns.
    • Verify the pronouns field is correctly saved in Core Data and displayed in the app.
  3. Localization:

    • Switch the app language to a non-English locale.
    • Confirm the "Pronouns" label appears translated where applicable.
  4. Migration Testing:

    • Run the app with a pre-Nos 20 database.
    • Verify that the migration to Nos 20 completes without errors and retains data integrity.

Technical Details

  • Core Data Migration: Requires a lightweight migration strategy to support the Nos 20 schema.
  • Localization Key: Added the key "pronouns" with the English translation "Pronouns" to Localizable.xcstrings.
  • Backward Compatibility: Safely handles cases where pronouns are missing in older metadata events or Core Data models.

Impact

  • New Feature: Enables users to share pronouns, improving inclusivity and personalization.
  • Codebase Changes: Touches Core Data models, metadata processing, and the profile editing flow.
  • Critical Migration: Ensures the database is migrated to Nos 20 without breaking existing functionality.

Code Review Checklist

  • Confirm migration path to Nos 20 is correctly implemented.
  • Verify Core Data model changes and lightweight migration logic.
  • Validate localization updates and backward compatibility.
  • Test pronouns integration in profile edit and metadata hydration.

@joshuatbrown
Copy link
Contributor

👀

Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

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

The code looks good overall. Unfortunately when I run this, set my pronouns, and save, they don't appear on my profile. When I go back to the Edit screen, they don't show up there, either. It seems like they get lost somewhere, but at this point I'm not sure where that is. I read all the code here on GitHub and didn't see where the issue is, so I guess it'll take a little debugging.

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 we can revert these changes; I don't think anything should need to change in version 19 of the data model.

Nos/Models/JSONEvent.swift Outdated Show resolved Hide resolved
NosTests/Models/CoreData/AuthorTests.swift Outdated Show resolved Hide resolved
NosTests/Models/CoreData/AuthorTests.swift Outdated Show resolved Hide resolved
@joshuatbrown
Copy link
Contributor

@rabble do you plan to also display the pronouns on the ProfileView and/or ProfileHeader?

@rabble rabble changed the title [WIP] Add Pronouns Support to Author Metadata and User Profiles Add Pronouns Support to Author Metadata and User Profiles Nov 26, 2024
@rabble rabble requested a review from joshuatbrown November 26, 2024 21:21
@joshuatbrown
Copy link
Contributor

👀

Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

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

One last suggestion; I'll go ahead and commit and merge. This looks great!

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

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

Excellent!

@joshuatbrown joshuatbrown added this pull request to the merge queue Nov 27, 2024
Merged via the queue into main with commit 36a61e9 Nov 27, 2024
4 checks passed
@joshuatbrown joshuatbrown deleted the pronouns branch November 27, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants