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

API to update a note #1013

Open
acomagu opened this issue Oct 14, 2018 · 3 comments
Open

API to update a note #1013

acomagu opened this issue Oct 14, 2018 · 3 comments
Labels
feature Wants to add a new feature

Comments

@acomagu
Copy link

acomagu commented Oct 14, 2018

It exists the API for creating note but doesn't for updating note.

@SISheogorath SISheogorath added the feature Wants to add a new feature label Oct 14, 2018
@SISheogorath
Copy link
Contributor

Updating a note, is actually a non-trivial thing, other than creating one.

codimd/lib/realtime.js

Lines 119 to 177 in d3ec67b

function updateNote (note, callback) {
models.Note.findOne({
where: {
id: note.id
}
}).then(function (_note) {
if (!_note) return callback(null, null)
// update user note history
var tempUsers = Object.assign({}, note.tempUsers)
note.tempUsers = {}
Object.keys(tempUsers).forEach(function (key) {
updateHistory(key, note, tempUsers[key])
})
if (note.lastchangeuser) {
if (_note.lastchangeuserId !== note.lastchangeuser) {
models.User.findOne({
where: {
id: note.lastchangeuser
}
}).then(function (user) {
if (!user) return callback(null, null)
note.lastchangeuserprofile = models.User.getProfile(user)
return finishUpdateNote(note, _note, callback)
}).catch(function (err) {
logger.error(err)
return callback(err, null)
})
} else {
return finishUpdateNote(note, _note, callback)
}
} else {
note.lastchangeuserprofile = null
return finishUpdateNote(note, _note, callback)
}
}).catch(function (err) {
logger.error(err)
return callback(err, null)
})
}
function finishUpdateNote (note, _note, callback) {
if (!note || !note.server) return callback(null, null)
var body = note.server.document
var title = note.title = models.Note.parseNoteTitle(body)
var values = {
title: title,
content: body,
authorship: note.authorship,
lastchangeuserId: note.lastchangeuser,
lastchangeAt: Date.now()
}
_note.update(values).then(function (_note) {
saverSleep = false
return callback(null, _note)
}).catch(function (err) {
logger.error(err)
return callback(err, null)
})
}

(And yes, here is space for internal optimizations)

So I don't see this happen in 1.3.0 but maybe in a future release.

@dsprenkels
Copy link
Contributor

Why not allow to use regular patch files for changes? Diff-match-patch should support that right? We could alternatively just allow anybody to update a note completely (and we can return the patch).

@ccoenen
Copy link
Contributor

ccoenen commented Jan 4, 2019

The problem is not with "how would someone provide a change" - both patch and "post it all" would accomplish that. The problem lies in the things that would have to follow from that change, namely at least: generating the OT version of that patch and distributing it to all connected parties and changing the note metadata.

I take @SISheogorath's comment to mean "patches welcome".

domq pushed a commit to epfl-fsd/hackmd.angularjs that referenced this issue Dec 5, 2019
- Abandon the idea of bidirectional sync, due to
hackmdio/codimd#1013

- Therefore, revert everything chokidar, and the `initialScan` of the
PostgreSQL database - We only track changes after the syncer starts
syncing

- Last command-line argument designates the directory to sync to

- AtomicCompareUpdateFile class that does the ole `O_WRONLY | O_CREAT
  | O_EXCL` dance to create a backup file, then uses
  `write-file-atomic` to update the main file, then deletes the backup
  file if and only if it is known to be useless (i.e. equal to the
  `oldContent` of the PostgreSQL event)

- Use a `defer()` + `concatAll()` combo to prevent multiple attempts to
save to the file system from occuring in parallel

- Touch up Dockerfile + docker-compose rig
edgarogh pushed a commit to WartaPoirier-corp/codimd that referenced this issue Sep 21, 2021
…ile-maintenance

Lock file maintenance (master)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Wants to add a new feature
Projects
None yet
Development

No branches or pull requests

4 participants