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

#surveyme in last Note comment should show the Note (even if there are no preceding comments) #6052

Closed
mnalis opened this issue Dec 27, 2024 · 4 comments

Comments

@mnalis
Copy link
Member

mnalis commented Dec 27, 2024

Back in #2641 (via b3ad39b), it was made that #surveyme tag in the last comment would always (unless manually hidden by the user) result in showing the OSM Note, even if that comment was created by the same user which is using the app.

In SC 60.0 it works mostly, i.e. if that comment is any but the first one.

I don't think that is intentional (I see no logic for that, nor do the comments seem to indicate the reason for such strange behaviour).
If the user's last comment contains a marker indicating the survey is required and we want to display the note in that case (as we do), then it should not matter how many comments preceded that last one (0 or more).

How to Reproduce

  1. add a comment to existing note with just text test; and note gets hidden - OK
  2. add a comment to existing note with text #surveyme test; and note remains displayed - OK
  3. create a new note with text #surveyme test -- note gets hidden - BAD

Expected Behavior

  • in step (3), note should remain displayed just as in does in (2), as the last message indicates the survey is required.

Versions affected
SC 60.0, Android 14

@kmpoppe
Copy link
Collaborator

kmpoppe commented Dec 29, 2024

This behaviour is coordinated by

private fun Note.shouldShowAsQuest(
userId: Long,
showOnlyNotesPhrasedAsQuestions: Boolean,
blockedNoteIds: Set<Long>
): Boolean {
// don't show notes hidden by user
if (id in blockedNoteIds) return false
// don't show notes where user replied last unless he wrote a survey required marker
if (comments.last().isReplyFromUser(userId)
&& !comments.last().containsSurveyRequiredMarker()
) {
return false
}
// newly created notes by user should not be shown if it was both created in this app and has no replies yet
if (probablyCreatedByUserInThisApp(userId) && !hasReplies) return false
/*
many notes are created to report problems on the map that cannot be resolved
through an on-site survey.
Likely, if something is posed as a question, the reporter expects someone to
answer/comment on it, possibly an information on-site is missing, so let's only show these
*/
if (showOnlyNotesPhrasedAsQuestions
&& !probablyContainsQuestion()
&& !containsSurveyRequiredMarker()
) {
return false
}
return true
}

more specifically

if (probablyCreatedByUserInThisApp(userId) && !hasReplies) return false

This hides a note that is created from (a) within SC, (b) by yourself and (c) has no replies, just like your test (3), no matter if #surveyme is contained or not.

A note I created via osm.org and downloaded the data in SC after that correctly shows the note, even though #surveyme is the first comment:

image

So, it would need to be discussed, whether the exclusion of notes from oneself within the app without replies should be hidden no matter what, or, if surveyme should "override" that behaviour.
IMHO, I can live with the behaviour as is.

@westnordost
Copy link
Member

Thanks for the investigation!

@westnordost
Copy link
Member

I'd agree that the current behavior is as intended. The reason is stated in the comment:

newly created notes by user should not be shown if it was both created in this app and has no replies yet

@westnordost westnordost closed this as not planned Won't fix, can't repro, duplicate, stale Dec 29, 2024
@westnordost westnordost removed the bug label Dec 29, 2024
@mnalis
Copy link
Member Author

mnalis commented Dec 29, 2024

So, it would need to be discussed, whether the exclusion of notes from oneself within the app without replies should be hidden no matter what, or, if surveyme should "override" that behaviour.

I see no reason why we should exclude them? #surveyme (especially if it has no following comments) clearly indicates that on-site survey is needed (And since the note was left in SC, also indicates that it cannot be surveyed on the ground at that moment, e.g. because being outside of opening hours or other limitations).

Original note creator can do that on-ground survey as well as any other on-the ground mapper (and probably better, as they obviously have active interest in that being solved, or they wouldn't create the note in the first place).

newly created notes by user should not be shown if it was both created in this app and has no replies yet

Yes, but that is pre-surveyme behaviour, and should be overriden by #surveyme?

Same as we regularly don't show the note if it was last commented by current user -- but that check is currently being overriden by #surveyme (and the same logic why it is overriden in case when it is second or third comment, should apply when it is a first comment).

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

No branches or pull requests

3 participants