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 support for isRead property for observable:MessageFacet #370

Open
1 of 10 tasks
chirag4semandex opened this issue Apr 27, 2022 · 5 comments · May be fixed by #371
Open
1 of 10 tasks

Add support for isRead property for observable:MessageFacet #370

chirag4semandex opened this issue Apr 27, 2022 · 5 comments · May be fixed by #371

Comments

@chirag4semandex
Copy link
Contributor

chirag4semandex commented Apr 27, 2022

Background

Currently, we have EmailMessageFacet, MessageFacet & SMSMessageFacet capturing the various different types of message details. The EmailMessageFacet and SMSMessageFacet has the observable:isRead property but MessageFacet is missing it. It makes sense to have this property there as well

Requirements

Requirement 1

Add observable:isRead to observable:MessageFacet.

Risk / Benefit analysis

Benefits

Adds support for capturing the read state of a given message that is NOT an Email OR SMS message

Risks

If there is a MessageFacet with a value of isRead that disagrees with the isRead value on, say, EmailMessageFacet, that will require significant explanation. It is not clear if this is something we should encode in the ontology as a violation.

Competencies demonstrated

Competency 1

This helps capture the missing property for various types of Messages like a Whatsapp message, it's not an Email and not an SMS message but it has the provenance that the user read the message or not and MessageFacet should allow capturing that.

Competency Question 1.1

Is a given message Read by the user?

Result 1.1

With this new field, The analytical tool can answer this question

Solution suggestion

  • add observable:isRead property to observable:MessageFacet
  • observable:isRead should not be removed from any other place

Coordination

  • Tracking in Jira ticket OC-233
  • Administrative review to be completed
  • Requirements to be discussed in Ontology Committee (OC) meeting, 2022-05-05.
  • Requirements Review vote has not occurred
  • Requirements development phase completed.
  • Solutions Approval to be discussed in OC meeting, date TBD
  • Solutions Approval vote has not occurred
  • Solutions development phase completed.
  • Implementation has not been merged into develop
  • Milestone linked
  • Documentation logged in pending release page
chirag4semandex added a commit to chirag4semandex/UCO that referenced this issue Apr 27, 2022
chirag4semandex added a commit to chirag4semandex/UCO that referenced this issue Apr 27, 2022
@ajnelson-nist ajnelson-nist linked a pull request Apr 27, 2022 that will close this issue
@ajnelson-nist
Copy link
Contributor

@chirag4semandex : there's some nuance to this proposal and subclass interactions with core:Facet. Background:

  • observable:EmailMessage is a subclass of observable:Message.
  • Properties that would be associated with a message are NOT stored on an object of class observable:Message, but are instead stored on an object of type observable:MessageFacet, and that facet object would be attached with core:hasFacet.
  • observable:EmailMessageFacet is NOT a subclass of observable:MessageFacet, but this is intentional. A subclass encoding between Facets would cause unpleasant behaviors with data needing to be repeated.

In ontologies that don't use something like UCO's core:Facet, this proposal would be reviewing usage of observable:isRead as a property of observable:EmailMessage. You proposed that isRead should be associated with general Messages, and if we adopted that change, suddenly it's redundant to specify an association with EmailMessage, SMSMessage, or any other subclass of Message.

However, UCO houses the property in Facets that are associated with Message subclasses. So, in spirit, to say isRead belongs to any Message, it's no longer appropriate to also house the property in EmailMessageFacet or SMSMessageFacet, because that's redundant.

All of the above should be apparent from review of UCO's design document section on Facets. If it is not, please say so, as the document is under community review and open to refinement.

Summarizing, I think there is an additional requirement that this proposal should have, but I'll leave it to you to add it if you agree:

  • Requirement 2: Facets associated with subclasses of Message (such as EmailMessageFacet) should no longer have the isRead property, due to redundancy. Current users of the property should migrate the property value to the ObservableObject's MessageFacet.

I also believe there is a risk, though it feels like a low-impact risk at first blush to me:

  • By designating isRead as a property associated with any Message or Message subclass (via MessageFacet), there is a design onus on future subclasses of Message to prevent usage of isRead when it is not applicable.

I don't have an example of this offhand, but I can imagine it applying in any sort of chat application that declines to use read-receipts.

@chirag4semandex
Copy link
Contributor Author

@ajnelson-nist, I see your point about EmailMessage being a subclass of Message and hence we should not have a duplicate property like isRead under the MessageFacet and also in EmailMessageFacet.

But then the question is: why do we have observable:from and observable:to properties duplicated between EmailMessageFacet and MessageFacet?

@chirag4semandex
Copy link
Contributor Author

My current problem is:

Lets say I have a Whatsapp message and I want to capture if it was read or not, the only way I have right now is to add a SMSMessageFacet and use the isRead property. Now that is clearly not correct since whatsapp message is not a SMSMessage and clearly I can not use EmailMessageFacet for it as well.

@ajnelson-nist
Copy link
Contributor

why do we have observable:from and observable:to properties duplicated between EmailMessageFacet and MessageFacet?

Now that you've pointed it out, that redundant assignment sounds like an error to me. Let's wait for a little more feedback before trying to reconcile this design. I suggest pushing forward here with consolidating isRead so we can settle on that policy by the next OC meeting.

@ajnelson-nist
Copy link
Contributor

The solution for this may entail renaming isRead to isMessageRead.

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 a pull request may close this issue.

2 participants